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 2 months 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.

Give us feedback!