NextGen - 0xAadi's results

Lines of code

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

Vulnerability details

Impact

The vulnerability in the auction implementation enables an attacker to steal the NFT through a two-step bidding strategy. Initially, the attacker places a bid with the minimum amount, followed by a second bid featuring an excessively large and undesirable amount. The purpose of the second bid is to prevent other users from surpassing the attacker's bid. Subsequently, the attacker can cancel the highest bid just before the auction concludes. Consequently, the attacker only needs to expend the minimum amount to seize the NFT, resulting in acquiring the NFT with a nominal cost.

Proof of Concept

Step 1: Attacker places the initial bid with the minimum amount participateToAuction(tokenId) with minAmount as msg.value

Step 2: Attacker places a second bid with a very large amount, this will prevent other users to participate the bidding. participateToAuction(tokenId) with largeAmount as msg.value

Step 3: Attacker cancels the highest bid just before the auction end time cancelBid(tokenId, index_of_highestBidId)

Step 4: Auction ends, and the attacker claims the auction with the minAmount claimAuction(tokenId)

Result: The attacker successfully acquires the NFT by only spending the minimum amount.

Tools Used

Manual Review

Recommended Mitigation Steps

Implement code to outbid the bidders previous bid while placing a new bid.

Assessed type

ERC721

Lines of code

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

Vulnerability details

Impact

The existing implementation of AuctionDemo permits malicious users to manipulate the bidding process. These users strategically place counter bids for each highest bid, inflating the NFT price. They then cancel all the bids just before the auction concludes. As a consequence, other bidders are compelled to pay inflated amounts to claim the NFT.

Proof of Concept

Step 1: Malicious user places counter bids to inflate the NFT price Malicious user participateToAuction(tokenId) with highestBidAmount + incrementAmount1 as msg.value Malicious user participateToAuction(tokenId) with highestBidAmount + incrementAmount2 as msg.value

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

Repeats when a user place a new bid.

Step 2: Malicious user cancels all the bids or the highest bid just before concluding the auction cancelAllBids()

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

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

Result: Other bidders need to pay high amounts to claim the NFT due to manipulated bids.

Tools Used

Manual Review

Recommended Mitigation Steps

Prevent users from canceling their bid if they are the highest bidder. This measure compels the highest bidder or any potential malicious user to proceed with claiming the NFT rather than canceling the bid.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

The NFT owner is unable to receive any Ether upon the conclusion of the auction. The code implemented in claimAuction() incorrectly transfers the winning bid amount to the contract owner rather than the NFT owner. This results in unfavorable circumstances within the auction process.

Proof of Concept

See line number 113, which transferring the highestBid to the contract owner, rather than ownerOfToken.

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

File: smart-contracts/AuctionDemo.sol

113: @>             (bool success, ) = payable(owner()).call{value: highestBid}("");
114:                emit ClaimAuction(owner(), _tokenid, success, highestBid);

Tools Used

Manual Review

Recommended Mitigation Steps

Use ownerOfToken instead of owner() in line number 113

         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}("");
+                (bool success, ) = payable(ownerOfToken).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}("");

Assessed type

ETH-Transfer