search

code-423n4 / 2022-08-nounsdao-findings

2 stars 0 forks source link

NounsDAOLogic.sol would become bricked if NoansDAOExecutor.sol admin ever changes #390

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/governance/NounsDAOExecutor.sol#L89-L95

Vulnerability details

Impact

NounsDAOLogic.sol would become bricked and upgradability would be completely broken

Proof of Concept

In the current setup, NoansDAOExecutor.sol is both admin and timelock for NounsDAOLogic.sol. If the admin for NoansDAOExecutor.sol was ever changed, NounsDAOLogic.sol would lose the ability to do anything, essentially bricking it and losing all the funds inside.

Tools Used

Recommended Mitigation Steps

The admin role in NoansDAOExecutor.sol#setPendingAdmin and acceptAdmin should be removed

davidbrai commented 2 years ago

this is a design choice that allows room for changing the DAO Logic contract of the DAOExecutor. changing that admin requires a proposal to pass so it would be very intentional