[Audit Report] NounsDAO

0xDave·2023년 1월 10일

Ethereum

목록 보기

84/112

주요 취약점

M-1: Unnecessary precision loss in _recipientBalance()

    function _recipientBalance() internal view returns (uint256) {
        uint256 startTime_ = startTime();
        uint256 blockTime = block.timestamp;

        if (blockTime <= startTime_) return 0;

        uint256 tokenAmount_ = tokenAmount();
        uint256 balance;
        if (blockTime >= stopTime()) {
            balance = tokenAmount_;
        } else {
            // This is safe because: blockTime > startTime_ (checked above).
            unchecked {
                uint256 elapsedTime_ = blockTime - startTime_;
                balance = (elapsedTime_ * ratePerSecond()) / RATE_DECIMALS_MULTIPLIER;
            }
        }

balance를 계산하는 식에서 불필요한 부분이 있다. 아래 ratePerSecond() 함수를 실행하게 되면

    function ratePerSecond() public pure returns (uint256) {
        uint256 duration = stopTime() - startTime();

        unchecked {
            return RATE_DECIMALS_MULTIPLIER * tokenAmount() / duration;
        }
    }

balance 계산식은 아래처럼 바꿀 수 있다. 이 때 RATE_DECIMALS_MULTIPLIER이 중복되므로 불필요한 계산이 추가되면서 가스비 소모로 이어진다.

balance = elapsedTime_ * (RATE_DECIMALS_MULTIPLIER * tokenAmount_ / duration) / RATE_DECIMALS_MULTIPLIER

따라서 다음과 같이 바꿔주는 것이 좋다.

balance = elapsedTime_ * tokenAmount_ / duration

M-2: Lack of sanity check for stoptime

    function createStream(
        address payer,
        address recipient,
        uint256 tokenAmount,
        address tokenAddress,
        uint256 startTime,
        uint256 stopTime,
        uint8 nonce
    ) public returns (address stream) {
        // These input checks are here rather than in Stream because these parameters are written
        // using clone-with-immutable-args, meaning they are already set when Stream is created and can't be
        // verified there. The main benefit of this approach is significant gas savings.
        if (payer == address(0)) revert PayerIsAddressZero();
        if (recipient == address(0)) revert RecipientIsAddressZero();
        if (tokenAmount == 0) revert TokenAmountIsZero();
        if (stopTime <= startTime) revert DurationMustBePositive();
        if (tokenAmount < stopTime - startTime) revert TokenAmountLessThanDuration();

        stream = streamImplementation.cloneDeterministic(
            encodeData(payer, recipient, tokenAmount, tokenAddress, startTime, stopTime),
            salt(
                msg.sender, payer, recipient, tokenAmount, tokenAddress, startTime, stopTime, nonce
            )
        );
        IStream(stream).initialize();

        emit StreamCreated(
            msg.sender, payer, recipient, tokenAmount, tokenAddress, startTime, stopTime, stream
            );
    }

stopTime이 현재 시간보다 뒤에 있다는 보장이 없다. 만약 현재 시간보다 이전 시간으로 된다면 단순히 토큰을 전송하는 기능을 할 뿐이다. 따라서 아래와 같은 코드가 추가되어야 한다.

 if (stopTime <= block.timestamp) revert StopTimeLessThanCurrentTime();

M-3: The rather harsh requirement of tokenAmount makes it inapplicable for certain tokens

    function createStream(
        address payer,
        address recipient,
        uint256 tokenAmount,
        address tokenAddress,
        uint256 startTime,
        uint256 stopTime,
        uint8 nonce
    ) public returns (address stream) {
        // These input checks are here rather than in Stream because these parameters are written
        // using clone-with-immutable-args, meaning they are already set when Stream is created and can't be
        // verified there. The main benefit of this approach is significant gas savings.
        if (payer == address(0)) revert PayerIsAddressZero();
        if (recipient == address(0)) revert RecipientIsAddressZero();
        if (tokenAmount == 0) revert TokenAmountIsZero();
        if (stopTime <= startTime) revert DurationMustBePositive();
        if (tokenAmount < stopTime - startTime) revert TokenAmountLessThanDuration();

		//..
    }

stream을 만들 때 마지막에 특이한 기준이 하나 있다. tokenAmount < stopTime - startTime 해당 기준을 만족하기 위해선 반드시 토큰의 decimal을 고려해야 한다. 만약 stream을 설정한 기준이 길다면 그에 따라 토큰의 양도 비례해서 늘어나야 한다. 이렇게 되면 decimal이 작은 토큰은 매우 큰 금액으로만 stream을 만들어야 한다. 1년을 기준으로 WBTC(decimals: 8)는 0.31536 WBTC per year ($5,400)를 충족해야 하며, EURS(decimals: 2)는 315,360 EURS를 충족해야 한다. 따라서 다음과 같이 수정하는 것이 좋다.

tokenAmount * RATE_DECIMALS_MULTIPLIER >= stopTime - startTime.

M-4: Two address tokens can be withdrawn by the payer even when the stream has began

토큰 중에는 여러 개의 주소를 가지고 있는 토큰이 있다. 예를 들어, Synthetix의 ProxyERC20 컨트랙트는 sUSD, sBTC 처럼 여러 주소를 가지고 있고 erc20 토큰과 같은 기능을 갖는다.

    function rescueERC20(address tokenAddress, uint256 amount) external onlyPayer {
        if (tokenAddress == address(token())) revert CannotRescueStreamToken();

        IERC20(tokenAddress).safeTransfer(msg.sender, amount);
    }