058.md

Lively Neon Scorpion

Medium

The payment arrangement may not align with the bidding winner's intention

Summary

The parameters immediateTreasuryBPs and streamLengthInTicks could be changed after a user wins an auction but before the auction is settled, the final payment arrangement may differ from the winner's original expectations.

Root Cause

The parameters immediateTreasuryBPs and streamLengthInTicks can be changed at any time.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

A user can bid an auction by calling NounsAuctionHouseV3#createBid() If the bidder wins the auction, part of the bid amount will be sent to the owner of NounsAuctionHouseV3 immediately:

336:        if (amountToSendTreasury > 0) {
337:            _safeTransferETHWithFallback(owner(), amountToSendTreasury);
338:        }

The remaining bid amount will be locked in StreamEscrow and paid to the DAO in streamLengthInTicks installments:

340:        if (amountToStream > 0) {
341:            streamEscrow.forwardAllAndCreateStream{ value: amountToStream }(_auction.nounId, streamLengthInTicks);
342:        } else {
343:            streamEscrow.forwardAll();
344:        }

However, the parameters immediateTreasuryBPs and streamLengthInTicks can be changed at any time. If a passed proposal to update these parameters is executed after a user wins an auction but before the auction is settled, the final payment arrangement may differ from the winner's original expectations.

Here is a example:

• immediateTreasuryBPs is 2000, streamLengthInTicks is 1000, and StreamEscrow.minimumTickDuration is 24 hours
• Alice bids 10 ETH for an auction of token 88. If she wins, 2 ETH will be sent to the treasury immediately, the remaining 8 ETH will be deposited in to StreamEscrow, and It will be vested to the treasury at a rate of up to 0.008 ETH per day.
• immediateTreasuryBPs is updated to 4000, and streamLengthInTicks is updated to 100.
• The auction is settled, and Alice win the auction. However 4 ETH is sent to the treasury immediately and the rest 6 ETH is deposited into StreamEscrow, which will be vested to the treasury at a rate of 0.06 ETH per day.
• Ninety days later, Alice planned to return token 88 and receive 7.28 ETH (8 / 1000 * 910). However, due to changes in the immediateTreasuryBPs and streamLengthInTicks parameters, she was only able to recover 0.6 ETH (6 / 100 * 10)

Impact

The payment arrangement may not align with the bidding winner's intention

PoC

No response

Mitigation

The sponsor has several ways to fix this issue:

• The simple solution is to keep both immediateTreasuryBPs and streamLengthInTicks as immutable.
• Another one is that neither immediateTreasuryBPs nor streamLengthInTicks should be updated while an auction is active.
• The third one is to cache immediateTreasuryBPs and streamLengthInTicks when creating a new auction:

+   uint16 public currentImmediateTreasuryBPs;
+   uint16 public currentStreamLengthInTicks;

    function _createAuction() internal {
        try nouns.mint() returns (uint256 nounId) {
            uint40 startTime = uint40(block.timestamp);
            uint40 endTime = startTime + uint40(duration);

            auctionStorage = AuctionV2({
                nounId: uint96(nounId),
                clientId: 0,
                amount: 0,
                startTime: startTime,
                endTime: endTime,
                bidder: payable(0),
                settled: false
            });

+           currentImmediateTreasuryBPs = immediateTreasuryBPs;
+           currentStreamLengthInTicks = streamLengthInTicks;

            emit AuctionCreated(nounId, startTime, endTime);
        } catch Error(string memory) {
            _pause();
        }
    }

    function _settleAuction() internal {
        INounsAuctionHouseV3.AuctionV2 memory _auction = auctionStorage;

        require(_auction.startTime != 0, "Auction hasn't begun");
        require(!_auction.settled, 'Auction has already been settled');
        require(block.timestamp >= _auction.endTime, "Auction hasn't completed");

        auctionStorage.settled = true;
-       uint256 amountToSendTreasury = (_auction.amount * immediateTreasuryBPs) / 10_000;
+       uint256 amountToSendTreasury = (_auction.amount * currentImmediateTreasuryBPs) / 10_000;
        uint256 amountToStream = _auction.amount - amountToSendTreasury;

        if (amountToSendTreasury > 0) {
            _safeTransferETHWithFallback(owner(), amountToSendTreasury);
        }

        if (amountToStream > 0) {
-           streamEscrow.forwardAllAndCreateStream{ value: amountToStream }(_auction.nounId, streamLengthInTicks);
+           streamEscrow.forwardAllAndCreateStream{ value: amountToStream }(_auction.nounId, currentStreamLengthInTicks);
        } else {
            streamEscrow.forwardAll();
        }

        if (_auction.bidder == address(0)) {
            nouns.burn(_auction.nounId);
        } else {
            nouns.transferFrom(address(this), _auction.bidder, _auction.nounId);
        }

        SettlementState storage settlementState = settlementHistory[_auction.nounId];
        settlementState.blockTimestamp = uint32(block.timestamp);
        settlementState.amount = ethPriceToUint64(_auction.amount);
        settlementState.winner = _auction.bidder;
        if (_auction.clientId > 0) settlementState.clientId = _auction.clientId;

        emit AuctionSettled(_auction.nounId, _auction.bidder, _auction.amount);
        if (_auction.clientId > 0) emit AuctionSettledWithClientId(_auction.nounId, _auction.clientId);
    }