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

[L-5] - `BidBeastsNFTMarket::placeBid` duplicate event emition and set of variales, causing confusion when placing a bid.

thecil

Root + Impact

[L-5] - BidBeastsNFTMarket::placeBid duplicate event emition and set of variales, causing confusion when placing a bid.

Description

The placeBid function is responsible to place a bid on an NFT listed in the market.

However, when an user only place a bid below the buyNowPrice of the listed NFT, the function is emitting the AuctionSettled event incorrectly, which can lead to confusion and potential issues for users. The AuctionSettled event should only be emitted when a bid is placed above or equal to the buyNowPrice.

Also, the placeBid function at his Buy Now Logic is setting the listing.listed = false variable, which is also beins set at the _executeSle function, which can lead to confusion and potential issues for users.

Also, the _executeSale function is also emitting the AuctionSettled event once the auction is settled.

The actual codebase of the _executeSale function:

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);
​
uint256 fee = (bid.amount * S_FEE_PERCENTAGE) / 100;
s_totalFee += fee;
uint256 sellerProceeds = bid.amount - fee;
_payout(listing.seller, sellerProceeds);
​
emit AuctionSettled(tokenId, bid.bidder, listing.seller, bid.amount);
}

This is the actual codebase for the placeBid function, we can see the AuctionSettled event is being emmited after the --- Buy Now Logic ---:

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");
​
// auctionEnd == 0 => no bids yet => allowed
// auctionEnd > 0 and block.timestamp >= auctionEnd => auction ended => block
require(listing.auctionEnd == 0 || block.timestamp < listing.auctionEnd, "Auction ended");
​
// --- Buy Now Logic ---
if (listing.buyNowPrice > 0 && msg.value >= listing.buyNowPrice) {
uint256 salePrice = listing.buyNowPrice;
uint256 overpay = msg.value - salePrice;
​
// EFFECT: set winner bid to exact sale price (keep consistent)
bids[tokenId] = Bid(msg.sender, salePrice);
@> listing.listed = false;
​
if (previousBidder != address(0)) {
_payout(previousBidder, previousBidAmount);
}
​
// NOTE: using internal finalize to do transfer/payouts. _executeSale will assume bids[tokenId] is the final winner.
_executeSale(tokenId);
​
// Refund overpay (if any) to buyer
if (overpay > 0) {
_payout(msg.sender, overpay);
}
​
return;
}
​
require(msg.sender != previousBidder, "Already highest bidder");
@> emit AuctionSettled(tokenId, msg.sender, listing.seller, msg.value);

Risk

Likelihood: Low.

Impact: Low.

Proof of Concept

(Proof Of Code)

  1. In the BidBeastsMkartePlaceTest.t.sol unit test file, place the following unit test:

function _placeBid(address user, uint256 tokenId, uint256 amount) private {
vm.startPrank(user);
market.placeBid{value: amount}(tokenId);
vm.stopPrank();
}
​
function test_eventAuctionSettledEmittedOnBids() public {
_mintNFT();
_listNFT();
_placeBid(BIDDER_1, TOKEN_ID, 2 ether);
_placeBid(BIDDER_2, TOKEN_ID, 3 ether);
}

  1. Run the unit test to demostrate the exploit.

forge test --mt test_eventAuctionSettledEmittedOnBids -vvvv

We can check the logs from the unit test transactions and we can realize that the event is being emitted when a bid made, instead of being emitted when the auction is completed.

BIDDER_1 logs when placeBid is call:

+ [72627] BidBeastsNFTMarket::placeBid{value: 2000000000000000000}(0)
+ - emit AuctionSettled(tokenId: 0, winner: RIPEMD-160: [0x0000000000000000000000000000000000000003], seller: SHA-256: [0x0000000000000000000000000000000000000002], price: 2000000000000000000 [2e18])
+ - emit AuctionExtended(tokenId: 0, newDeadline: 901)
+ - emit BidPlaced(tokenId: 0, bidder: RIPEMD-160: [0x0000000000000000000000000000000000000003], amount: 2000000000000000000 [2e18])

BIDDER_2 logs when placeBid is call:

+ [15891] BidBeastsNFTMarket::placeBid{value: 3000000000000000000}(0)
+ - emit AuctionSettled(tokenId: 0, winner: Identity: [0x0000000000000000000000000000000000000004], seller: SHA-256: [0x0000000000000000000000000000000000000002], price: 3000000000000000000 [3e18])
+ - [600] PRECOMPILES::ripemd{value: 2000000000000000000}(0x)
+ - - ← [Return] 0x0000000000000000000000009c1185a5c5e9fc54612808977ee8f548b2258d31
+ - emit BidPlaced(tokenId: 0, bidder: Identity: [0x0000000000000000000000000000000000000004], amount: 3000000000000000000 [3e18])

Recommended Mitigation

The AuctionSettled event should be emitted after the auction is completed.

Since the event is being emitted on the _executeSale function, we can just remove it from the placeBid function.

Also you can remove the listing.listed = false code line, since it is also being set at the _executeSale function.

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!