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

2 stars 0 forks source link

Not checking for `address(0)` value for `admin` in proxy constructor #320

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOProxyV2.sol#L74

Vulnerability details

Impact

Setting the admin to address(0) will block the whole "only-admin" functionallity

_setImplementation
_setVotingDelay
_setVotingPeriod
_setProposalThresholdBPS
_setMinQuorumVotesBPS
_setMaxQuorumVotesBPS
_setQuorumCoefficient
_setDynamicQuorumParams
_withdraw
_setPendingAdmin)

Proof of Concept

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOProxyV2.sol#L74

Tools Used

VSCode, Slither

Recommended Mitigation Steps

Consider safety-checking if (admin_ == address(0)) in NounsDAOProxyV2.constructor

davidbrai commented 2 years ago

While this is true, I think this is a non critical issue.