The winner can DoS the auction and stuck the NFT and users funds in the contract

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Winners will be able to DoS auctions and lock both their NFTs and all of the funds of other bidders. This issue arises because AuctionDemo's claimAuction uses push instead of a pull.

The winner does not need to be a user, it can also be a competitive entity. A competitor against the collection or against NextGen.

Proof of Concept

Once the auction has ended, the winner can invoke claimAuction. And to revert on his _checkOnERC721Received.

The core issue is in this if if, within this for loop, where, if the current bidder is not the winner, their funds are refunded.

for (uint256 i=0; i< auctionInfoData[_tokenid].length; i++) {
    ...
    } 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 {}
}

Example:

1. NFT is scheduled for auction with start of 0.01ETH
2. 50 more bidders bid - 0.011, 0.015... 0.29.
3. Attacker bids last - 0.3ETH (1 wei higher than the bid before him) using a contract that reverts when it receives the NFT.
4. Auction ends.
5. claimAuction reverts as the call made to the attacker contract reverts.

Attacker loses some ETH, however 50 bidders lose their bids (the last one is the same as the attaker) and the NFT will remain stuck in the contract.

Tools Used

Manual review

Recommended Mitigation Steps

Utilize the pull method instead of the push method. This can be achieved by creating another function that allows users to manually withdraw their funds after the auction concludes.

Assessed type

DoS

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

[c4-judge]

alex-ppg marked the issue as partial-50

[c4-judge]

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