Root + Impact
Description
-
The Auctions are supposed to be active for 3 days after the first bid - Explained in the Docs. With the current logic this is not possible because the Auctions end when no bids are placed 15 min after the first bid.
-
This creates a wrong behaviour:
- Auctions can end just 15 min after the first bid if nobody bid in this time.
- Missleading information for the behaviour of the contract.
-
The S_AUCTION_EXTENSION_DURATION constant is set to 15 min and it's used to extend the duration, but this is not the desired behaviour
function placeBid(uint256 tokenId) external payable isListed(tokenId) {
More code...
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);
} else {
requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
require(msg.value >= requiredAmount, "Bid not high enough");
uint256 timeLeft = 0;
if (listing.auctionEnd > block.timestamp) {
timeLeft = listing.auctionEnd - block.timestamp;
}
@> if (timeLeft < S_AUCTION_EXTENSION_DURATION) {
@> listing.auctionEnd = listing.auctionEnd + S_AUCTION_EXTENSION_DURATION;
emit AuctionExtended(tokenId, listing.auctionEnd);
}
}
More code...
}
Risk
Likelihood:
Impact:
Proof of Concept
The test test_bidAfterOneDay will Fail due to the logic that makes the Auction end 15 after the first bid.
function test_bidAfterOneDay() public {
_mintNFT();
_listNFT();
uint256 newBid = MIN_PRICE + 10;
vm.prank(BIDDER_1);
market.placeBid{value: newBid}(TOKEN_ID);
vm.warp(block.timestamp + 1 days);
uint256 secondBidAmount = newBid * 120 / 100;
vm.prank(BIDDER_2);
market.placeBid{value: secondBidAmount}(TOKEN_ID);
}
Recommended Mitigation
We need to replace the S_AUCTION_EXTENSION_DURATION with S_AUCTION_DEADLINE and change the time from 15 minutes to 3 days
in the placeBid function to replace the constant in the if statement which handles 1st bid and remove the check for updating the auctionEnd .
After this changes the auction will end just after the 3rd day like it's expected by the Docs.
// on line 34
- uint256 constant public S_AUCTION_EXTENSION_DURATION = 15 minutes;
+ uint256 public constant S_AUCTION_DEADLINE = 3 days;
function placeBid(uint256 tokenId) external payable isListed(tokenId) {
More code...
// --- 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 + S_AUCTION_DEADLINE;
emit AuctionExtended(tokenId, listing.auctionEnd);
} else {
requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
require(msg.value >= requiredAmount, "Bid not high enough");
- uint256 timeLeft = 0; //<@ This lines are not needed as far as we will not update the auctionEnd anymore
- if (listing.auctionEnd > block.timestamp) {
- timeLeft = listing.auctionEnd - block.timestamp;
- }
- if (timeLeft < S_AUCTION_EXTENSION_DURATION) {
- listing.auctionEnd = listing.auctionEnd + S_AUCTION_EXTENSION_DURATION;
- emit AuctionExtended(tokenId, listing.auctionEnd);
- }
}
More code...
}
