Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

Prefix - Lender.sol and Marketplace.sol admin can be set to any address #49

Closed
sherlock-admin opened this issue Nov 10, 2022 · 0 comments
Labels

Comments

@sherlock-admin
Copy link
Contributor

Prefix

low

Lender.sol and Marketplace.sol admin can be set to any address

Summary

Admin address can be set to zero by mistake, thus disabling all the admin methods.

Vulnerability Detail

The method setAdmin only sets the admin address without any checks:
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L219-L223

This means that if admins use this method mistakingly with address that they do not own, they would lose the admin access to the marketplace forever. Because README.md says that admins are always multisig, probability of such mistake is smaller but it is still there.

The same problem is repeated in Marketplace.sol.

Impact

Losing administration access to a marketplace.

Code Snippet

Tool used

Manual Review

Recommendation

Copied from point 50 of https://secureum.substack.com/p/security-pitfalls-and-best-practices-101 :
Changing critical addresses in contracts should be a two-step process where the first transaction (from the old/current address) registers the new address (i.e. grants ownership) and the second transaction (from the new address) replaces the old address with the new one (i.e. claims ownership). This gives an opportunity to recover from incorrect addresses mistakenly used in the first step. If not, contract functionality might become inaccessible. (see here and here)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

1 participant