search

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

0 stars 1 forks source link

Malicious Owner can steal all user funds #16

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L159-L172

Vulnerability details

Submitting as med risk because it would require malicious multisig, but there should never be absolute trust in any party especially when there's no reason fees would ever need to be that high anyways

Impact

Owner steals all of user funds

Proof of Concept

EntryFees and ExitFees can both be set to 100% (10000 bps) meaning that owner could set fees to 100% and collect all user funds as "fees". There would be a timelock to allow some users to leave before this goes into effect but I find it highly unlikely that 1) the transaction will be noticed and 2) that all users would remove their funds from before the 7 day timelock was complete. Additionally emergency access allows an immediate action which could be used to steal all user funds without notice if the multisig colludes.

Tools Used

Recommended Mitigation Steps

Both fees should be limited to a reasonable amount such as 10% (1000 bps)

obatirou commented 2 years ago

Disputed

“I find it highly unlikely that 1) the transaction will be noticed and 2) that all users would remove their funds from before the 7 day timelock was complete.”

This is not an issue. It's about wardens appreciation of our ownership architecture versus ours. We can imagine many other malicious scenarios, assuming that the Multisig/Timelock/OwnerProxy combination is not enough to prevent the protocol from being compromised.