NextGen - ak1's results

Lines of code

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

Vulnerability details

Impact

Winner / highest bidder can drain the contract funds.

Highest bidder can steal the NFT with minimal loss

Proof of Concept

AuctionDemo would be used to place bid on a particular NFT. whenever a bid is placed, the function participateToAuction is called and bid is placed.

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L57-L61

    function participateToAuction(uint256 _tokenid) public payable {
        require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);
        auctionInfoStru memory newBid = auctionInfoStru(msg.sender, msg.value, true);
        auctionInfoData[_tokenid].push(newBid);
    }

This function will get the last highest bided value using the function returnHighestBid and checks with current bid amount. The auction should be active (checks the end time).

Once the auction is over, either the winner or admin can call the function claimAuction and claim the NFT. The amount will be sent to owner.

If the bidder wants to cancel the bid, they can call the function cancelBid or cancelAllBids to cancel and the bided amount is sent to them.

The issue here is the checks that is used to validate whether the bid is active or not ( within the auction end time)

Let see the codes.

    function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
        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);
        address highestBidder = returnHighestBidder(_tokenid);
        for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) {
            if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) {
                IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
                (bool success, ) = payable(owner()).call{value: highestBid}("");
                emit ClaimAuction(owner(), _tokenid, success, highestBid);
            } else if (auctionInfoData[_tokenid][i].status == true) {
                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}(""); -------------------------------------->>> refund first time
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {}
        }
    }

    // cancel a single Bid

    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}(""); -------------------->>> refund second time
        emit CancelBid(msg.sender, _tokenid, index, success, auctionInfoData[_tokenid][index].bid);
    }

Both functions have a common check require(block.timestamp >= minter.getAuctionEndTime(_tokenid). Please note the equal sign.

In the same block , the highest bidder can call the claimAuction and cancelBid. Note the block.timestamp would be same.

First, highest bidder can call the function claimAuction when block.timestamp == minter.getAuctionEndTime(_tokenid). Note, this function , refund the amount to the bidder if the bid amount is not highest bid (refer the else block). This is the place where the bidder will get the amount first time.

Also, the function cancelBid will be called by this highest bidder. when we see the check require(block.timestamp <= minter.getAuctionEndTime(_tokenid) ---->> note the equal sign. So the, check will allow to cancel one time and refund the amount once again.

To steak the NFT with minimal loss, they will call the functions claimAuction and cancelBid sequentially in the same block where block.timestamp would be same when calling these functions.

Tools Used

Manual review.

Recommended Mitigation Steps

We suggest to update the auctionInfoData[_tokenid][index].status = false; inside the claim function as shown below.

    function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
        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);
        address highestBidder = returnHighestBidder(_tokenid);
        for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) {
            if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) {
                IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
                (bool success, ) = payable(owner()).call{value: highestBid}("");
                emit ClaimAuction(owner(), _tokenid, success, highestBid);
                auctionInfoData[_tokenid][index].status = false; ----------------------------->>>++++
            } else if (auctionInfoData[_tokenid][i].status == true) {
                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
                auctionInfoData[_tokenid][index].status = false; ---------------------------------->> ++++
            } else {}
        }
    }

Assessed type

ETH-Transfer

First, highest bidder can call the function claimAuction when block.timestamp == minter.getAuctionEndTime(_tokenid). Note, this function , refund the amount to the bidder if the bid amount is not highest bid (refer the else block). This is the place where the bidder will get the amount first time.

Also, the function cancelBid will be called by this highest bidder.
when we see the check  require(block.timestamp <= minter.getAuctionEndTime(_tokenid) ---->> note the equal sign.
So the, check will allow to cancel one time and refund the amount once again.

To steak the NFT with minimal loss, they will call the functions claimAuction and cancelBid sequentially in the same block where block.timestamp would be same when calling these functions.