Invariant Protocol Break, anyone cannot finalize auction after 3 days
Description
-
Anyone can finalize the auction after 3 days, according to the contest detail "After 3 days, anyone can call endAuction(tokenId) to finalize the auction." is designed to prevent the auction from taking too long even listing.auctionEnd > 3 days.
-
Unfortunately, in function BidBeastsNFTMarket::settleAuction() there is no checks that if an auction has been ongoing for 3 days. The function only checks for listing.auctionEnd which is its dynamic deadline and it can exceed more than 3 days if there's many bidders come in.
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:
Impact:
Proof of Concept
Simplified for testing:
uint256 constant public S_AUCTION_EXTENSION_DURATION = 4 days;
copy and paste the PoC to BidBeastMarketPlaceTest.t.sol
function test_auctionCannotFinalizeAfter3days() public {
_mintNFT();
_listNFT();
vm.prank(BIDDER_1);
market.placeBid{value: MIN_PRICE + 1}(TOKEN_ID);
BidBeastsNFTMarket.Listing memory list = market.getListing(TOKEN_ID);
assertEq(4 days + 1, list.auctionEnd);
vm.warp(block.timestamp + 3 days);
vm.expectRevert();
vm.prank(BIDDER_2);
market.settleAuction(TOKEN_ID);
}
the log:
[332591] BidBeastsNFTMarketTest::test_auctionCannotFinalizeAfter3days()
├─ [0] VM::startPrank(ECRecover: [0x0000000000000000000000000000000000000001])
│ └─ ← [Return]
├─ [74067] BidBeasts::mint(SHA-256: [0x0000000000000000000000000000000000000002])
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: SHA-256: [0x0000000000000000000000000000000000000002], tokenId: 0)
│ ├─ emit BidBeastsMinted(to: SHA-256: [0x0000000000000000000000000000000000000002], tokenId: 0)
│ └─ ← [Return] 0
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(SHA-256: [0x0000000000000000000000000000000000000002])
│ └─ ← [Return]
├─ [25508] BidBeasts::approve(BidBeastsNFTMarket: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 0)
│ ├─ emit Approval(owner: SHA-256: [0x0000000000000000000000000000000000000002], approved: BidBeastsNFTMarket: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], tokenId: 0)
│ └─ ← [Stop]
├─ [128588] BidBeastsNFTMarket::listNFT(0, 1000000000000000000 [1e18], 5000000000000000000 [5e18])
│ ├─ [1094] BidBeasts::ownerOf(0) [staticcall]
│ │ └─ ← [Return] SHA-256: [0x0000000000000000000000000000000000000002]
│ ├─ [29510] BidBeasts::transferFrom(SHA-256: [0x0000000000000000000000000000000000000002], BidBeastsNFTMarket: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 0)
│ │ ├─ emit Transfer(from: SHA-256: [0x0000000000000000000000000000000000000002], to: BidBeastsNFTMarket: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], tokenId: 0)
│ │ └─ ← [Stop]
│ ├─ emit NftListed(tokenId: 0, seller: SHA-256: [0x0000000000000000000000000000000000000002], minPrice: 1000000000000000000 [1e18], buyNowPrice: 5000000000000000000 [5e18])
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::prank(RIPEMD-160: [0x0000000000000000000000000000000000000003])
│ └─ ← [Return]
├─ [72627] BidBeastsNFTMarket::placeBid{value: 1000000000000000001}(0)
│ ├─ emit AuctionSettled(tokenId: 0, winner: RIPEMD-160: [0x0000000000000000000000000000000000000003], seller: SHA-256: [0x0000000000000000000000000000000000000002], price: 1000000000000000001 [1e18])
│ ├─ emit AuctionExtended(tokenId: 0, newDeadline: 345601 [3.456e5])
│ ├─ emit BidPlaced(tokenId: 0, bidder: RIPEMD-160: [0x0000000000000000000000000000000000000003], amount: 1000000000000000001 [1e18])
│ └─ ← [Stop]
├─ [2122] BidBeastsNFTMarket::getListing(0) [staticcall]
│ └─ ← [Return] Listing({ seller: 0x0000000000000000000000000000000000000002, minPrice: 1000000000000000000 [1e18], buyNowPrice: 5000000000000000000 [5e18], auctionEnd: 345601 [3.456e5], listed: true })
├─ [0] VM::warp(259201 [2.592e5])
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error 0xf4844814)
│ └─ ← [Return]
├─ [0] VM::prank(Identity: [0x0000000000000000000000000000000000000004])
│ └─ ← [Return]
├─ [1311] BidBeastsNFTMarket::settleAuction(0)
│ └─ ← [Revert] Auction has not ended
└─ ← [Stop]
Recommended Mitigation
so track auctionStart in BidBeastsNFTMarket::listNFT() function and in BidBeastsNFTMarket::settleAuction() checks if block.timestamp >= auctionStart + 3 days, anyone can finalize, even though auctionEnd is still in the future due to extension.
append auctionStart in struct Listing:
struct Listing {
address seller;
uint256 minPrice;
uint256 buyNowPrice;
+ uint256 auctionStart;
uint256 auctionEnd;
bool listed;
}
create new constant variable:
+ uint256 constant public S_MAX_AUCTION_DURATION = 3 days;
update BidBeastsNFTMarket::listNFT() function:
function listNFT(uint256 tokenId, uint256 _minPrice, uint256 _buyNowPrice) external {
....
listings[tokenId] = Listing({
seller: msg.sender,
minPrice: _minPrice,
buyNowPrice: _buyNowPrice,
+ auctionStart: block.timestamp,
auctionEnd: 0, // Timer starts only after the first valid bid.
listed: true
});
emit NftListed(tokenId, msg.sender, _minPrice, _buyNowPrice);
}
update BidBeastsNFTMarket::settleAuction() function:
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");
+ bool reachedDeadline = block.timestamp >= listing.auctionEnd;
+ bool reachedMaxDuration = block.timestamp >= (listing.auctionStart + S_MAX_AUCTION_DURATION);
+ require(reachedDeadline || reachedMaxDuration, "Auction has not ended");
require(bids[tokenId].amount >= listing.minPrice, "Highest bid did not meet min price");
_executeSale(tokenId);
}
