Nouns DAO contest - yixxas's results

Lines of code

https://github.com/code-423n4/2022-08-nounsdao/blob/c1c7c6201d0247f92472419ff657b570f9104565/contracts/governance/NounsDAOLogicV2.sol#L839

Vulnerability details

Impact

_setVetoer() is extremely important to the security of this contract as the vetoer holds the power to stop any proposal from being executed. If vetoer is set to an address without an owner, it can open up the protocol to governance attack as explained by Dialectic. If vetoer is set to a malicious actor, all funds in the protocol will be stuck in contract as all proposals can be vetoed.

Proof of Concept

_setVetoer() does not check that newVetoer address is not the 0 address nor require the target address to accept this transfer of vetoer.

NounsDAOLogicV2.sol#L839-L845

    function _setVetoer(address newVetoer) public {
        require(msg.sender == vetoer, 'NounsDAO::_setVetoer: vetoer only');

        emit NewVetoer(vetoer, newVetoer);

        vetoer = newVetoer;
    }

Recommended Mitigation Steps

Consider adding an acceptVetoer() function similar to the one required for changing of admin.