Bid Beasts
First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: medium
Invalid

CEI pattern not followed, so a bidder can outbid themself

farnad

Root + Impact

Description

  • Since the ckeck for not being the previous bidder is located at a wrong place, a bidder which currently is the previousBidder can pay the BidBeastsNFTMarket::Listings::buyNowPrice and buy the NFT. This way, they have placed 2 consecutive bids and outbidded themself which is a contradiction case for an auction.

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");
​

Risk

Likelihood:

  • This happens when a bidder plaaces a bid for an NFT, but before anyone else places a bid for that NFT, the same bidder pays the BidBeastsNFTMarket::Listings::buyNowPrice and buys the NFT. It can happen fairly often.

Impact:

  • The bidder places 2 consecutive bids, and outbids themself. This is a contradiction to the auction rules.

Proof of Concept

Place this function in the test file and run it.

function test_PlaceTwoConsecutiveBids() public {
// Arrange
_mintNFT();
_listNFT();
//Act
vm.startPrank(BIDDER_1);
market.placeBid{value: MIN_PRICE + 1}(TOKEN_ID);
market.placeBid{value: BUY_NOW_PRICE}(TOKEN_ID);
vm.stopPrank();
//Assert
assertEq(nft.ownerOf(TOKEN_ID), BIDDER_1);
}
​

Recommended Mitigation

The check (require) statement needs to be moved to before the if statement.

+ require(msg.sender != previousBidder, "Already highest bidder");
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");
​
Updates

Lead Judging Commences

cryptoghost Lead Judge 26 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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