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 25 days 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.