Bid Beasts
First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

[H-3] Auction end logic in README not enforced in contract logic, invalidating the protocol's intended functionality

cyberethdev

[H-3] Auction end logic in README not enforced in contract logic, invalidating the protocol's intended functionality

Description

  • Normal bahaviour: According to the README "After 3 days, anyone can call endAuction(tokenId) to finalize the auction.".

  • Problematic behaviour: The 3 day auction duration threshold is not enforced in the contract logic. In addition, the endAuction(tokenId) function does not exist. The equivalent contract method is BidBeastsNFTMarketPlace::settleAuction. A user can settle the auction as soon as the auctionEnd has passed.

Root cause:
As seen in the code snippet below, the auction end is set to the block timestamp plus the S_AUCTION_EXTENSION_DURATION constant variable, which is set to 15 minutes in the contract.

...
// --- Regular Bidding Logic ---
uint256 requiredAmount;
​
if (previousBidAmount == 0) {
requiredAmount = listing.minPrice;
require(msg.value > requiredAmount, "First bid must be > min price");
@> listing.auctionEnd = block.timestamp + S_AUCTION_EXTENSION_DURATION;
emit AuctionExtended(tokenId, listing.auctionEnd);
...

Risk

Likelihood: High

  • The logic error occurs for every listed NFT. The auction end will be set to 15 minutes in the future whenever a first bid is placed.

Impact: High

  • All auctions in the NFT marketplace will have an initial duration of 15 minutes instead of 3 days, breaking the protocol's logic.

Proof of Concept

The following test proves that the auction can be settled when 15 minutes have passed after the first bid, assuming that no other bid has been placed. This contradicts the protocol logic outlined in the README, which states that auctions can be settled after 3 days have passed.

To run the test, include it in the Foundry test suite and test it with forge test --mt test_placeBid_AuctionCanEndAfter15Mins.

function test_placeBid_AuctionCanEndAfter15Mins() public {
_mintNFT();
_listNFT();
​
// Place first bid
vm.startPrank(BIDDER_1);
uint256 bidPrice = MIN_PRICE * 2;
market.placeBid{ value: bidPrice }(TOKEN_ID);
​
// Check that auction end is set to 15 minutes in the future
BidBeastsNFTMarket.Listing memory listing = market.getListing(TOKEN_ID);
uint256 auctionEnd = listing.auctionEnd;
uint256 expectedAuctionEnd = block.timestamp + market.S_AUCTION_EXTENSION_DURATION();
assertEq(auctionEnd, expectedAuctionEnd, "Auction end is not 15 minutes in the future");
​
// Fast-forward time by 16 minutes
vm.warp(block.timestamp + 16 minutes);
​
// Check that the settleAuction method can be called
vm.expectEmit(true, true, true, true);
emit AuctionSettled(TOKEN_ID, BIDDER_1, SELLER, bidPrice);
// This should succeed as the auction duration has passed
market.settleAuction(TOKEN_ID);
​
vm.stopPrank();
}

Recommended Mitigation

To mitigate the logic error, a new constant variable should be added to the contract with the initial auction duration:

+ uint256 constant public AUCTION_INITIAL_DURATION = 3 days;

Solution A:
When the first bid on a listed NFT is placed, the auction end should be set as follows:

...
// --- Regular Bidding Logic ---
uint256 requiredAmount;
​
if (previousBidAmount == 0) {
requiredAmount = listing.minPrice;
require(msg.value > requiredAmount, "First bid must be > min price");
- listing.auctionEnd = block.timestamp + S_AUCTION_EXTENSION_DURATION;
+ listing.auctionEnd = block.timestamp + AUCTION_INITIAL_DURATION;
emit AuctionExtended(tokenId, listing.auctionEnd);
...

Solution B:
Set the auctionEnd to block timestamp + AUCTION_INITIAL_DURATION when the NFT is listed in the marketplace:

function listNFT(uint256 tokenId, uint256 _minPrice, uint256 _buyNowPrice) external {
require(BBERC721.ownerOf(tokenId) == msg.sender, "Not the owner");
require(_minPrice >= S_MIN_NFT_PRICE, "Min price too low");
if (_buyNowPrice > 0) {
require(_minPrice <= _buyNowPrice, "Min price cannot exceed buy now price");
}
​
BBERC721.transferFrom(msg.sender, address(this), tokenId);
​
listings[tokenId] = Listing({
seller: msg.sender,
minPrice: _minPrice,
buyNowPrice: _buyNowPrice,
- auctionEnd: 0, // Timer starts only after the first valid bid.
+ auctionEnd: block.timestamp + AUCTION_INITIAL_DURATION,
listed: true
});
​
emit NftListed(tokenId, msg.sender, _minPrice, _buyNowPrice);
}
​

This solution requires a re-work of the logic in the contract whenever the auctionEnd variable is used in a contract method.

Finally, the endAuction(tokenId) method in the README should be corrected to settleAuction(tokenId).

Updates

Lead Judging Commences

cryptoghost Lead Judge
about 1 month ago
cryptoghost Lead Judge 29 days ago
Submission Judgement Published
Validated
Assigned finding tags:

BidBeasts Marketplace: Improper Documentation

Documentation for BidBeasts Marketplace is incomplete or inaccurate, potentially leading to misconfigurations or security misunderstandings.

Support

FAQs

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