NextGen - max10afternoon's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/71d055b623b0d027886f1799739b7f785b5bc7cd/smart-contracts/AuctionDemo.sol#L124

Vulnerability details

Impact

The auctionDemo contract enables users to cancel and withdraw any bid that they previously made, including the current highest bid. This means that a user can make a completely out of market bid completely discouraging other users from bidding again (eg: if an NFT from a collection is worth on average 10, they can bid for 50) and than cancel the bid at the last second, meaning that the auction will be virtually unavailable for the vast majority of the time.

This opens up for following scenarios:

1. As the auction start a user can bid an out of market amount, wait till the last few seconds, than cancel the bid, preventing the majority of users from participating in the auction, likely getting the item sold for a discounted price, as the time left for bidding will be minimum.

2. As the auction start a user can bid an out of market amount, wait till the last few seconds, than cancel the bid, than places a very low bid and (even 1 wei, if possible), if afterwards there is no time left (or extremely little) they might get the item for the specified extremely cheap price.

3. An artist can artificially pump the price of their assets, without incurring in any risk, as they can always withdraw their highest bid (even if no one surpass them), to let the one immediately below win (frontrunnig withdraws if necessary).

4. A validator knowing that they will mine blocks around the auction's end timestamp, can do the same trick of bidding extremely high, cutting away any competition, and then both delete the bid an create a new one, inside the block where the timestamp is equal to the end of the auction, getting the NFT at an artificially discounted price.

Proof of Concept

-1,2,3)

The auctionDemo contract, which enables NFT to be auctioned, has a cancelBid function, that lets users delete a bid and withdraw correspondent ETH:

    function cancelBid(uint256 _tokenid, uint256 index) public {
        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
        require(auctionInfoData[_tokenid][index].bidder == msg.sender && auctionInfoData[_tokenid][index].status == true);
        auctionInfoData[_tokenid][index].status = false;
        (bool success, ) = payable(auctionInfoData[_tokenid][index].bidder).call{value: auctionInfoData[_tokenid][index].bid}("");
        emit CancelBid(msg.sender, _tokenid, index, success, auctionInfoData[_tokenid][index].bid);
    }

As can be seen this function only checks that: -block.timestamp <= minter.getAuctionEndTime(_tokenid), the auction hasn't ended. -auctionInfoData[_tokenid][index].bidder == msg.sender, msg.sender is the one who placed the bid. -uctionInfoData[_tokenid][index].status == true and that the bid has not been already withdrawn.

This means that even the highest bid can be withdrawn, since there is no restriction against it, enabling the scenarios listet in Impact.

-4) As can be observed the checks against the auction's end performed by participateToAuction, cancelBid and claimAuction, the three functions necessary to place a new bid, delete an old one, and claim the price, are all performed using <= or >=:

    function participateToAuction(uint256 _tokenid) public payable {
        require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);

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

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

Meaning that they can all three be invoked in the same block for as long as block.timestamp == minter.getAuctionEndTime(_tokenid), enabling scenario 4, in the impact session

Recommended Mitigation Steps

the cancelBid function could check that the bid that is being deleted, isn't the currently highest bid

Assessed type

Other