Bid Beasts
First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: high
Invalid

[M-2] The NFT is not returned to the original seller after the auction end if no bids were made, breaking the protocol's intended functionality

cyberethdev

[M-2] The NFT is not returned to the original seller after the auction end if no bids were made, breaking the protocol's intended functionality

Description

  • Normal behaviour: When 3 days since the NFT listing have passed and no bids have been made in that time period, the NFT should be returned back to the Seller.

  • Problematic behaviour: The 3 day initial auction duration is not currently enforced in the contract logic. In addition, the NFT is not automatically returned to the seller unless the seller calls the BidBeastsNFTMarketPlace::unlistNFT method.

Root cause:
The function below settles the auction for NFTs that had bidding activity and whose auctionEnd has passed, and does not return the NFT to the seller when no valid bids have been placed:

function settleAuction(uint256 tokenId) external isListed(tokenId) {
Listing storage listing = listings[tokenId];
@> require(listing.auctionEnd > 0, "Auction has not started (no bids)");
require(block.timestamp >= listing.auctionEnd, "Auction has not ended");
require(bids[tokenId].amount >= listing.minPrice, "Highest bid did not meet min price");
​
_executeSale(tokenId);
}

Risk

Likelihood: High

  • This oversight affects every listed NFT that has not received any valid bids.

Impact: Medium

  • Even though this logic error does not have any financial impact, it does break the intended protocol logic which dictates that NFTs will be returned automatically to their sellers when an auction has been settled.

Proof of Concept

The following test shows that settling an auction when more than 3 days have passed since the NFT listing does not return the NFT to the seller.

Run the test with forge test --mt test_settleAuction_DoesNotReturnNFTToSellerIfNoBids.

function test_settleAuction_DoesNotReturnNFTToSellerIfNoBids() public {
_mintNFT();
_listNFT();
​
// Fast-forward time by 4 days
vm.warp(block.timestamp + 4 days);
​
// Check that the settleAuction method reverts as no bids were placed
vm.prank(SELLER);
vm.expectRevert("Auction has not started (no bids)");
market.settleAuction(TOKEN_ID);
​
// Check that the NFT is still owned by the marketplace contract
address nftOwner = nft.ownerOf(TOKEN_ID);
assertEq(nftOwner, address(market), "The NFT should still be owned by the marketplace contract");
}

Recommended Mitigation

To address this logic error:
A. add the 3 day auction duration logic in the contract:

  • See reported vulnerability H-3

B. Add an internal function that transfers the NFT back to the seller:

+ function _returnNFT(uint256 tokenId) internal {
+ require(bids[tokenId].bidder == address(0), "Cannot unlist, a bid has been placed");
+ Listing storage listing = listings[tokenId];
+ listing.listed = false;
​
+ BBERC721.transferFrom(address(this), listing.seller, tokenId);
​
+ emit NFTUnlisted(tokenId);
+ }

C. Integrate the _returnNFT function in the settleAuction logic. However, this first requires addressing the reported logic error as shown in H-3.

Updates

Lead Judging Commences

cryptoghost Lead Judge 25 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.