Griefing attacks on NounsAuctionHouse

[code423n4]

Lines of code

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/NounsAuctionHouse.sol#L257

Vulnerability details

Impact

There is internal function _safeTransferETH that is called in createBid.

The function itself:

    function _safeTransferETH(address to, uint256 value) internal returns (bool) {
        (bool success, ) = to.call{ value: value, gas: 30_000 }(new bytes(0));
        return success;
    }

Please note that

(bool success, ) = to.call{ value: value, gas: 30_000 }(new bytes(0));

is the same as

(bool success, bytes memory data) = to.call{ value: value, gas: 30_000 }(new bytes(0));

So, basically, the fact that the bytes memory are missed does not mean that there are no of it. In other words, the data that was returned by call will be copied into the memory of the contract execution. That means the contract can consume as much gas as this copying will cost (with the allocation of new memory).

Memory allocation has O(n^2) gas cost. That's mean that more data you already allocate that more expensive allocation will be, and the cost growing strongly.

That's being said _safeTransferETH in createBid, specifically it is used to return the bet to the previous bidder. So, the malicious bidder can create a contract that will return a lot of data on fallback function. It would lead to high gas consumption on latter call to createBid.

Proof of Concept

// SPDX-License-Identifier: GPL-3.0

pragma solidity >=0.5.0 <0.9.0;

contract PoC {
    address maliciousUser;

    constructor() public {
        maliciousUser = address(new MaliciousUser());
    }

    // Call to that function consume 95k of gas.
    function test() external {
        (bool success, ) = maliciousUser.call{ gas: 30_000 }(new bytes(0));
    }
}

contract MaliciousUser {
    fallback() external payable {
        assembly { 
            return(0, 103168) // revert(0, 103168)
        }
    }
}

Recommended Mitigation Steps

Use low-level inline assembly call.

        bool success;
        assembly {
            success := call(30000, addr, amount, 0, 0, 0)
        }

[davidbrai]

Thanks for reporting this issue. We plan to fix in on our next upgrade of the NounsAuctionHouse.

According to our analysis this is capped by gas usage you reported due to only 30K gas passed in the call.

I believe this contract was out of scope for this contest. If this will not be compensation through the contest, please reach out to us on discord for compensation

[gzeoneth]

out of scope