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

[L-1] The `BidBeastsNFTMarket::takeHighestBid` function is missing a period check, creating some level of unclear information and confusion for users.

alexscherbatyuk

The BidBeastsNFTMarket::takeHighestBid function is missing a period check, creating some level of unclear information and confusion for users.

Description

  • The BidBeastsNFTMarket::takeHighestBid function’s NatSpec documentation indicates it can only be called before the auction ends. However, it does not check if the auction has ended, allowing the seller to take the highest bid after the auction ends.

  • This creates some level of unclear information and confusion for users, as it partially repeats the functionality of the `BidBeastsNFTMarket::settleAuction` function.

function takeHighestBid(uint256 tokenId) external isListed(tokenId) isSeller(tokenId, msg.sender) {
// @>
Bid storage bid = bids[tokenId];
require(bid.amount >= listings[tokenId].minPrice, "Highest bid is below min price");
​
_executeSale(tokenId);
}

Risk

Likelihood: Medium

  • It might occur under specific conditions.

  • Seller accidentally calls takeHighestBid after an auction ends.

Impact: Low

  • Incorrect use of function.

  • Not a standard flow.

Proof of Concept

Add the following code snippet to the `BidBeastsMarketPlaceTest.t.sol` test file.

This code snippet is designed to demonstrate the `BidBeastsNFTMarket::takeHighestBid` function being successfully called after the auction ends.

function testTakeHighestBidAfterAuctionEnd() public {
vm.startPrank(OWNER);
nft.mint(SELLER);
vm.stopPrank();
​
vm.startPrank(SELLER);
nft.approve(address(market), TOKEN_ID);
market.listNFT(TOKEN_ID, MIN_PRICE, BUY_NOW_PRICE);
vm.stopPrank();
​
// First bid to start auction
uint256 bidAmount = MIN_PRICE + 1;
vm.prank(BIDDER_1);
market.placeBid{value: bidAmount}(TOKEN_ID);
​
// End auction and settle
vm.warp(block.timestamp + market.S_AUCTION_EXTENSION_DURATION() + 1);
vm.prank(SELLER);
market.takeHighestBid(TOKEN_ID);
​
assertEq(nft.ownerOf(TOKEN_ID), BIDDER_1);
assertEq(market.getListing(TOKEN_ID).listed, false);
}

Recommended Mitigation

Add a check for the auction end time to the `BidBeastsNFTMarket::takeHighestBid` function to ensure it can only be called before the auction ends.

function takeHighestBid(uint256 tokenId) external isListed(tokenId) isSeller(tokenId, msg.sender) {
+ require(listings[tokenId].auctionEnd > block.timestamp, "Auction has ended");
Bid storage bid = bids[tokenId]; // creates a link to bids[tokenId] value, why ?
require(bid.amount >= listings[tokenId].minPrice, "Highest bid is below min price");
​
_executeSale(tokenId);
}
Updates

Lead Judging Commences

cryptoghost Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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