Bid Beasts

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

Incorrect `AuctionSettled` event emission

Root + Impact

Description

  • The AuctionSettled event should be emitted only if the auction is settled and the NFT is unlisted, as in _executeSale()

  • However, the placeBid() function emits the AuctionSettled event before the bid logic, which is wrong and confusing

  • Indexers may see a sequence of an auction's event like: settled -> extended -> settled (shown in PoC)

function placeBid(uint256 tokenId) external payable isListed(tokenId) {
...
// --- Buy Now Logic ---
if (listing.buyNowPrice > 0 && msg.value >= listing.buyNowPrice) {
...
}
require(msg.sender != previousBidder, "Already highest bidder");
@> // shouldn't emit AuctionSettled event before the auction is settled
emit AuctionSettled(tokenId, msg.sender, listing.seller, msg.value);
// --- Regular Bidding Logic ---
uint256 requiredAmount;
if (previousBidAmount == 0) {
...
} else {
...
}
...
emit BidPlaced(tokenId, msg.sender, msg.value);
}

Risk

Likelihood: High

  • It always triggers when placeBid() is called

  • Emission happens unconditionally before bid logic, so any bid reproduces it

Impact: Medium

  • Misleading state

  • Event sequence inconsistency: Seeing two AuctionSettled events in one auction (one false, one real) breaks downstream assumptions of monotonic auction states, making integration unreliable and debugging costly.

  • Automation errors: Off-chain processes triggered by AuctionSettled may run too early.

  • Data corruption: Trading volume, fee, and royalty analytics may double-count or mis-record transactions.

Proof of Concept

Add the following test, then run this command: forge test --match-test test_catch_AuctionSettled_event

function test_catch_AuctionSettled_event() public {
_mintNFT();
_listNFT();
vm.recordLogs();
vm.prank(BIDDER_1);
market.placeBid{value: MIN_PRICE + 1 ether}(TOKEN_ID);
vm.prank(SELLER);
market.takeHighestBid(TOKEN_ID);
Vm.Log[] memory entries = vm.getRecordedLogs();
assertEq(entries.length, 5);
assertEq(entries[0].topics[0], keccak256(("AuctionSettled(uint256,address,address,uint256)")));
assertEq(entries[1].topics[0], keccak256(("AuctionExtended(uint256,uint256)")));
assertEq(entries[2].topics[0], keccak256(("BidPlaced(uint256,address,uint256)")));
assertEq(entries[3].topics[0], keccak256(("Transfer(address,address,uint256)")));
assertEq(entries[4].topics[0], keccak256(("AuctionSettled(uint256,address,address,uint256)")));
}

Recommended Mitigation

Directly remove the AuctionSettled event emission inside the placeBid(), only emit it if the auction is settled (i.e., inside _executeSale()).

function placeBid(uint256 tokenId) external payable isListed(tokenId) {
...
// --- Buy Now Logic ---
if (listing.buyNowPrice > 0 && msg.value >= listing.buyNowPrice) {
...
}
require(msg.sender != previousBidder, "Already highest bidder");
- emit AuctionSettled(tokenId, msg.sender, listing.seller, msg.value);
// --- Regular Bidding Logic ---
uint256 requiredAmount;
if (previousBidAmount == 0) {
...
} else {
...
}
...
emit BidPlaced(tokenId, msg.sender, msg.value);
}
Updates

Lead Judging Commences

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

BidBeasts Marketplace: Incorrect Event Emission

placeBid emits AuctionSettled even though the auction hasn’t ended, causing misleading event logs.

Support

FAQs

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