Digital garden

code4rena-2022-08-nounsdao-h01

6 min read

code4rena-2022-08-nounsdao-h01

[H-01] ERC721Checkpointable: delegateBySig allows the user to vote to address 0, which causes the user to permanently lose his vote and cannot transfer his NFT.

보고서

Summary

파라미터가 값이 address(0) 인지 확인하지 않아 문제가 발생했다. delegateBySig 를 통해 delegatee address(0) 에게 표를 위임한 경우 유저가 소유한 투표권이 전부 사라지며, 유저는 더이상 자신의 NFT를 transfer/burn 할 수 없게 된다.

Keyword

input validation, logic flaw, dao, delegate vote, checkpoint, dos

Vulnerability

    function delegate(address delegatee) public {
        if (delegatee == address(0)) delegatee = msg.sender;
        return _delegate(msg.sender, delegatee);
    }

기본적인 투표권 위임 기능 함수인 delegate 는 위와 같다. delegate 파라미터로 address(0) 가 파라미터로 들어오는 경우 기존 투표권 위임을 취소하고 위임했던 표를 되돌려 받는다.

    function delegateBySig(
        address delegatee,
        uint256 nonce,
        uint256 expiry,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) public {
        bytes32 domainSeparator = keccak256(
            abi.encode(DOMAIN_TYPEHASH, keccak256(bytes(name())), getChainId(), address(this))
        );
        bytes32 structHash = keccak256(abi.encode(DELEGATION_TYPEHASH, delegatee, nonce, expiry));
        bytes32 digest = keccak256(abi.encodePacked('\x19\x01', domainSeparator, structHash));
        address signatory = ecrecover(digest, v, r, s);
        require(signatory != address(0), 'ERC721Checkpointable::delegateBySig: invalid signature');
        require(nonce == nonces[signatory]++, 'ERC721Checkpointable::delegateBySig: invalid nonce');
        require(block.timestamp <= expiry, 'ERC721Checkpointable::delegateBySig: signature expired');
        return _delegate(signatory, delegatee);
    }

이와 유사하게 delegateBySig 함수는 유저의 서명을 받아 파라미터로 넣는, 유저가 직접 컨트랙트콜하지 않는 형식으로 투표권 위임을 할 수 있는 기능을 제공한다. 이 때, delegate 함수와는 다르게 delegatee 파라미터가 address(0) 인지 확인하는 로직이 빠져있다. 즉, address(0) 를 delegatee 파라미터로 받은 경우 기존의 투표권 위임을 취소해야 하는데, 그게 아니라 address(0) 에게 위임하는 꼴이 된다.

    function _delegate(address delegator, address delegatee) internal {
        /// @notice differs from `_delegate()` in `Comp.sol` to use `delegates` override method to simulate auto-delegation
        address currentDelegate = delegates(delegator);
 
        _delegates[delegator] = delegatee;
 
        emit DelegateChanged(delegator, currentDelegate, delegatee);
        // @audit-info 현재 가진 모든 NFT balance (인데 96bit로 자른)
        uint96 amount = votesToDelegate(delegator);
        // @audit-info 소유한 NFT 개수만큼 표 delegate 
        _moveDelegates(currentDelegate, delegatee, amount);
    }
 
    function delegates(address delegator) public view returns (address) {
        address current = _delegates[delegator];
        return current == address(0) ? delegator : current;
    }
 
    function _moveDelegates(
        address srcRep,
        address dstRep,
        uint96 amount
    ) internal {
        if (srcRep != dstRep && amount > 0) {
            if (srcRep != address(0)) {
                // @audit-info transfer 또는 burn
                uint32 srcRepNum = numCheckpoints[srcRep];
                uint96 srcRepOld = srcRepNum > 0 ? checkpoints[srcRep][srcRepNum - 1].votes : 0;
                uint96 srcRepNew = sub96(srcRepOld, amount, 'ERC721Checkpointable::_moveDelegates: amount underflows');
                _writeCheckpoint(srcRep, srcRepNum, srcRepOld, srcRepNew);
            }
 
            if (dstRep != address(0)) {
                // @audit-info transfer 또는 mint
                uint32 dstRepNum = numCheckpoints[dstRep];
                uint96 dstRepOld = dstRepNum > 0 ? checkpoints[dstRep][dstRepNum - 1].votes : 0;
                uint96 dstRepNew = add96(dstRepOld, amount, 'ERC721Checkpointable::_moveDelegates: amount overflows');
                _writeCheckpoint(dstRep, dstRepNum, dstRepOld, dstRepNew);
            }
        }
    }

_delegate(signatory, delegatee) 를 호출했을때 플로우를 살펴보자.

  1. address currentDelegate = delegates(delegator); 로 현재 delegatee를 알아냄. 이전에 위임하지 않았다면 delegator 자신
  2. _moveDelegates(currentDelegate, delegatee, amount); 를 호출하여 delegator → delegatee 로 표 이동. 이 때, delegatee 가 address(0)
  3. _moveDelegates 내에서 dstRep 가 address(0) 이므로, burn과 동일하게 처리됨. srcRep의 표는 줄어들지만 dstRep의 표가 늘지는 않음(address(0) 이기 때문)

즉, 이 표는 사라지게 된다.

    function _beforeTokenTransfer(
        address from,
        address to,
        uint256 tokenId
    ) internal override {
        super._beforeTokenTransfer(from, to, tokenId);
 
        /// @notice Differs from `_transferTokens()` to use `delegates` override method to simulate auto-delegation
        _moveDelegates(delegates(from), delegates(to), 1);
    }
 
    function delegates(address delegator) public view returns (address) {
        address current = _delegates[delegator];
        return current == address(0) ? delegator : current;
    }
 
    function _moveDelegates(
        address srcRep,
        address dstRep,
        uint96 amount
    ) internal {
        if (srcRep != dstRep && amount > 0) {
            if (srcRep != address(0)) {
                // @audit-info transfer 또는 burn
                uint32 srcRepNum = numCheckpoints[srcRep];
                uint96 srcRepOld = srcRepNum > 0 ? checkpoints[srcRep][srcRepNum - 1].votes : 0;
                uint96 srcRepNew = sub96(srcRepOld, amount, 'ERC721Checkpointable::_moveDelegates: amount underflows');
                _writeCheckpoint(srcRep, srcRepNum, srcRepOld, srcRepNew);
            }
            ...
        }
    }

투표권을 위임한 유저가 NFT를 transfer 하는 상황을 생각해보자.

  1. 투표권을 address(0) 에게 위임했기 때문에 delegates(from)는 from이다.
  2. 이미 투표권을 위임한 from 에게는 투표권이 없어 _moveDelegates 를 호출했을 때 numCheckpoints[srcRep] 값이 0이 된다.
  3. uint96 srcRepNew = sub96(srcRepOld, amount, 'ERC721Checkpointable::_moveDelegates: amount underflows'); 에서 언더플로우가 발생해 revert 된다.

이 유저는 투표권 계산 중 언더플로우로 인해 NFT를 transfer 할 수 없게 된다. 동일한 이유로 burn도 할 수 없다.

Impact

위임한 투표권이 사라지며, 투표권을 위임한 유저가 자신의 NFT를 transfer하거나 burn할 수 없게 된다.

Mitigation

delegateBySig 에 require(delegatee != address(0)); 를 추가하거나, address(0) 인 경우 delegate 함수와 동일하게 투표권 위임을 취소하는 기능을 구현한다.

tags: bughunting, nouns dao, dao, dao delegate, smart contract, solidity, lack-of-input-validation-vul, snapshot, erc721, logic flaw, severity high

Graph View