search

code-423n4 / 2022-06-illuminate-findings

1 stars 0 forks source link

[M-04] Admin can lose control on the contract #401

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/marketplace/MarketPlace.sol#L109-L112

Vulnerability details

Proof of Concept

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/marketplace/MarketPlace.sol#L109-L112

The setAdmin function is too loose, if no one have access to the address passed in this function, the whole project is stuck - cannot withdraw funds or fees, cannot create new markets, etc.

Recommended Mitigation Steps

Change setAdmin in favour of a two steps control transfer - where the new anointed admin must complete a transaction from the new address, before the control is actually transferred.

sourabhmarathe commented 2 years ago

Bugs resulting from lack of input validation by a user or admin are beyond the scope of the audit.

EDIT: This is a duplicate of the other issues that pointed out that the admin setter should follow a two-step process. As a result, I am updating this ticket to be a duplicate of #316 and marking it as "Sponsor Acknowledged".

JTraversa commented 2 years ago

To clarify, the setX methods are initialized and never touched again, specifically the setAdmin is gated behind a multisig where all input is validated.

So in reality even if there are ever any issues with initialization, no funds are at risk, we just redeploy the contracts.