Nouns DAO contest - bin2chen's results

Lines of code

https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/base/ERC721Checkpointable.sol#L143 https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/base/ERC721Checkpointable.sol#L219

Vulnerability details

Impact

delegateBySig() no check address(0) ,malicious users can lock other user's NFT when call delegateBySig(delegatee=address(0)) , the delegatee is still self, but user checkpoints.votes will be reduced. it will cause the NFT of the user being delegate to be locked, can no set other delegatee or NFT transfer

Proof of Concept

Example: step 1. user_A has 1 NFT, user_B has 1 NFT, and user_B's delegatee is user_A, then user_A's Checkpoints.votes=2 step 2. user_A call delegateBySig(address(0)), after user_A 's Checkpoints.votes=1 step 3. user_A transfers its own NFT to user_C, then user_A 's Checkpoints.votes=0 step 4. then user_B can not modify the delegatee, NFT can not be transferred,be lock

    function delegateBySig(
        address delegatee,
        uint256 nonce,
        uint256 expiry,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) public {
         .....

        //****delegatee can be address(0)***/
        return _delegate(signatory, delegatee); 
    }

when 1,2,3 steps later, user_A's Checkpoints.votes=0 If user_B wants to transfer NFT or set a new delegatee, it will call _moveDelegates(),because srcRep = user_A, and the user_A's Checkpoints.votes=0, minus 1 will underflow and cause failure

    function _moveDelegates(
        address srcRep,
        address dstRep,
        uint96 amount
    ) internal {
        if (srcRep != dstRep && amount > 0) {
            if (srcRep != address(0)) {
                uint32 srcRepNum = numCheckpoints[srcRep];
                uint96 srcRepOld = srcRepNum > 0 ? checkpoints[srcRep][srcRepNum - 1].votes : 0;
                //***** 0-1 underflows *****//
                uint96 srcRepNew = sub96(srcRepOld, amount, 'ERC721Checkpointable::_moveDelegates: amount underflows');
                _writeCheckpoint(srcRep, srcRepNum, srcRepOld, srcRepNew);
            }

        }
    }

Tools Used

Recommended Mitigation Steps

check delegatee!=address(0)