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

Misleading settlement event: placeBid() emits AuctionSettled during bidding (pre-settlement), corrupting off-chain logic and accounting

berkep1914

Root + Impact

Description

  • Normal behavior: During bidding, contracts should emit bid-related events (e.g., BidPlaced, AuctionExtended). The settlement event should be emitted only once the auction is finalized (after NFT transfer and payments), typically inside the settlement routine.

  • Issue: placeBid() emits AuctionSettled immediately after basic checks, before any settlement logic and even before updating the highest bid, which incorrectly signals that the auction has concluded. Off-chain consumers (indexers, bots, dashboards) that rely on AuctionSettled for final prices/owners/fees will be misled, potentially triggering early payouts, incorrect accounting, or broken UX. _executeSale() later emits AuctionSettled again, compounding confusion (duplicate/conflicting settlements).

function placeBid(uint256 tokenId) external payable isListed(tokenId) {
Listing storage listing = listings[tokenId];
address previousBidder = bids[tokenId].bidder;
uint256 previousBidAmount = bids[tokenId].amount;
​
require(listing.seller != msg.sender, "Seller cannot bid");
require(listing.auctionEnd == 0 || block.timestamp < listing.auctionEnd, "Auction ended");
​
// @> BUG: Settlement event during bidding (pre-finalization)
emit AuctionSettled(tokenId, msg.sender, listing.seller, msg.value);
​
// ... bidding logic continues, not a real settlement ...
// later, _executeSale(tokenId) emits AuctionSettled again (the real one)
}
​

Risk

Likelihood:

  • Occurs on every call to placeBid(), including early bids, buy-now attempts that might later revert, or bids that won’t win the auction.

Off-chain systems that subscribe to AuctionSettled will process false positives frequently.

Impact:

  • Incorrect off-chain state & accounting: Indexers record a “settled” sale for msg.value that isn’t final; bots may prematurely distribute proceeds or update ownership/leaderboards wrongly.

Operational/UX breakage: Double settlement signals (one fake from placeBid, one real from _executeSale) create data ambiguity and maintenance burdens for integrators.

Proof of Concept

1) User calls placeBid(tokenId) with a normal bid.

2) Contract immediately emits: AuctionSettled(tokenId, bidder, seller, msg.value) -> Off-chain services mark auction as settled at 'msg.value'.

3) Auction continues (more bids or time extension).

4) Later, _executeSale() runs and emits AuctionSettled again with the final price.

Result: off-chain data shows conflicting/duplicate settlements; early one is wrong.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import {Test, console} from "forge-std/Test.sol";
import {BidBeastsNFTMarket} from "../src/BidBeastsNFTMarketPlace.sol";
import {BidBeasts} from "../src/BidBeasts_NFT_ERC721.sol";
​
contract MisleadingSettlementEventPoC is Test {
BidBeastsNFTMarket market;
BidBeasts nft;
​
address OWNER = address(0xA1);
address SELLER = address(0xB2);
address BIDDER = address(0xC3);
​
uint256 constant TOKEN_ID = 0;
uint256 constant MIN_PRICE = 1 ether;
​
event AuctionSettled(uint256 tokenId, address winner, address seller, uint256 price);
​
function setUp() public {
vm.startPrank(OWNER);
nft = new BidBeasts();
market = new BidBeastsNFTMarket(address(nft));
vm.stopPrank();
​
vm.deal(SELLER, 100 ether);
vm.deal(BIDDER, 100 ether);
​
// mint & list
vm.prank(OWNER);
nft.mint(SELLER);
​
vm.startPrank(SELLER);
nft.approve(address(market), TOKEN_ID);
market.listNFT(TOKEN_ID, MIN_PRICE, 0);
vm.stopPrank();
}
​
function test_SettlementEventEmittedDuringBid_shouldNotHappen() public {
// BUG DEMO: placeBid emits AuctionSettled even though no settlement happened
vm.prank(BIDDER);
​
vm.expectEmit(true, true, true, true);
emit AuctionSettled(TOKEN_ID, BIDDER, SELLER, MIN_PRICE); // ← emitted inside placeBid()
market.placeBid{value: MIN_PRICE}(TOKEN_ID);
}
}
​

Recommended Mitigation

Notes

  • Keep one settlement signal (in _executeSale) as the canonical source of truth.

  • Consider adding explicit lifecycle events (NftListed, AuctionExtended, NftUnlisted)—you already have them; ensure semantics stay phase-accurate.

  • Optionally switch to safeTransferFrom in _executeSale to avoid transfers to non-receiver contracts (design choice).

function placeBid(uint256 tokenId) external payable isListed(tokenId) {
Listing storage listing = listings[tokenId];
address previousBidder = bids[tokenId].bidder;
uint256 previousBidAmount = bids[tokenId].amount;
​
require(listing.seller != msg.sender, "Seller cannot bid");
require(listing.auctionEnd == 0 || block.timestamp < listing.auctionEnd, "Auction ended");
​
- // Wrong semantic: settlement event during bidding
- emit AuctionSettled(tokenId, msg.sender, listing.seller, msg.value);
​
// --- Buy Now Logic ---
if (listing.buyNowPrice > 0 && msg.value >= listing.buyNowPrice) {
...
_executeSale(tokenId); // emits AuctionSettled once, at the true settlement point
...
return;
}
​
// bidding logic (start/extend auction, compute required amount, etc.)
...
- emit BidPlaced(tokenId, msg.sender, msg.value);
+ // Emit only bid-phase signals here
+ emit BidPlaced(tokenId, msg.sender, msg.value);
}
​
function _executeSale(uint256 tokenId) internal {
Listing storage listing = listings[tokenId];
Bid memory bid = bids[tokenId];
​
listing.listed = false;
delete bids[tokenId];
​
- BBERC721.transferFrom(address(this), bid.bidder, tokenId);
+ // (Optional hardening) Use safeTransferFrom to prevent token lock on non-receiver contracts
+ BBERC721.transferFrom(address(this), bid.bidder, tokenId);
​
uint256 fee = (bid.amount * S_FEE_PERCENTAGE) / 100;
s_totalFee += fee;
uint256 sellerProceeds = bid.amount - fee;
_payout(listing.seller, sellerProceeds);
​
// Single authoritative settlement event here only
emit AuctionSettled(tokenId, bid.bidder, listing.seller, bid.amount);
}
​
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!