NextGen - Shubham's results

Lines of code

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

Vulnerability details

The auctionDemo contract holds the current auctions after the mintAndAuction functionality is called. Users can bid on a token and the highest bidder can claim the token after an auction finishes.

Users call participateToAuction() to participate & place their bid in a auction & they can only participate if their bid is higher than the highest bid for that item. Once the auction finishes, the person with the highest bid & if their status is active/true can call claimAuction() to claim their item.

However there is no restriction on what the starting price for an auction should be which makes it very easy for the attacker to win the auction & claim the prize but just giving some dust amount (1 wei) thereby causing a huge loss to the artist & organization.

Proof of Concept

Bidders call participateToAuction() & if msg.value > returnHighestBid(_tokenid) passes during the auction duration, they become the highest bidder until another buyer outbids their bid.

File: AuctionDemo

    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);
    }

claimAuction() has a modifier to ensure who can call the function i.e. the highest bidder, function admin or global admin to claim the NFT which goes to the highest bidder.

File: AuctionDemo

    modifier WinnerOrAdminRequired(uint256 _tokenId, bytes4 _selector) {
      require(msg.sender == returnHighestBidder(_tokenId) || adminsContract.retrieveFunctionAdmin(msg.sender, _selector) == true || adminsContract.retrieveGlobalAdmin(msg.sender) == true, "Not allowed");
      _;
    }

Also a bidder can cancel their bid by calling cancelBid() which changes their bidding status to false.

File: AuctionDemo

    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;        /// @note - status changes to 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);
    }

Attack Scenario:

Although only one account is enough to carry out the attack but we are assuming attacker is using two accounts for better clarity.

• Bob is the attacker who has two accounts X & Y with 1 wei & a huge amount respectively.
• As soon as the auction begins, Bob calls participateToAuction() with account X having 1 wei.
• It passes because auctionInfoData[_tokenid].length is empty & returnHighestBid(_tokenid) returns 0 in this case.
• Bob has now entered the bid with account X.
• Bob immediately calls participateToAuction() with account Y which has a very huge amount of money.
• Bob now has both accounts X & Y consecutively in auctionInfoData.
• A new bidder wants to participate but will not be able to since the highest bid amount exceeds what the user is willing to pay & so he is unable to enter the bid.
• Bob waits till the end & as soon as the auction is about to end, he calls cancelBid() with the address of account Y.
• Active status now changes to false for the account Y.
• The auction ends & now there are only two bidder which are both Bob's account. One with 1 wei (which is active) & the other with a huge sum of money (whose status is now cancelled/false).
• Bob calls claimAuction() which now checks the highest bid amount & bidder which is 1 wei & Bob respectively.
• Function enters the loop & the 1st IF condition returns true.
• NFT is transferred from the owner to Bob & the owner receive the highest bid amount that is 1 wei.

File: AuctionDemo

    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);    /// @note - Returns 1 wei
        address ownerOfToken = IERC721(gencore).ownerOf(_tokenid);
        address highestBidder = returnHighestBidder(_tokenid);   /// @note - returns Bob
        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) {    /// @note - check passes
                IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);  /// @note - NFT is received to Bob
                (bool success, ) = payable(owner()).call{value: highestBid}("");  /// @note - Owner is paid 1 wei
                emit ClaimAuction(owner(), _tokenid, success, highestBid);
        .........

Impact

Attacker can claim NFTs with dust amount causing a complete lost to the artist & the protocol.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a minimum starting auction amount to avoid this situation.

Assessed type

Error