The winner of a certain auction may extort other participating users

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

In the auctionDemo contract, the claimAuction function can be called by the winner or the administrator to transfer the NFT to the winner and return the ETH to the remaining users who have not yet cancelled the auction.
If the winner's address is a contract, the contract can perform a malicious rollback when receiving the NFT, locking many people's ETH in the contract and making it impossible to withdraw.
Malicious users can extort money through this kind of behavior.

Proof of Concept

Let's take a deep dive into the claimAuction function:

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}("");
            emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
        } else {}
    }
}

Note that if the statement IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid); occurs revert, the entire transaction will also fail to complete. Continuing to dive deeper into the safeTransferFrom function, you will find the following code:

function _safeTransfer(address from, address to, uint256 tokenId, bytes memory data) internal virtual {
    _transfer(from, to, tokenId);
    require(_checkOnERC721Received(from, to, tokenId, data), "ERC721: transfer to non ERC721Receiver implementer");
}

If a malicious contract is maliciously revert in it's onERC721Received callback function, the attack can be successfully completed.

Tools Used

Foundry And VsCode

Recommended Mitigation Steps

Use try catch statements to optimize NFT safeTransferFrom behavior

Assessed type

DoS

[c4-pre-sort]

141345 marked the issue as duplicate of #245

[c4-pre-sort]

141345 marked the issue as not a duplicate

[c4-pre-sort]

141345 marked the issue as duplicate of #1653

[c4-pre-sort]

141345 marked the issue as duplicate of #843

[c4-pre-sort]

141345 marked the issue as duplicate of #486

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #1759

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

alex-ppg changed the severity to 2 (Med Risk)