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

Incorrect Required Price Validation in First Bid

objectplayer

Root + Impact

Description

  • The normal behavior of a bidding system should ensure that minimum bid requirements are applied consistently throughout the auction lifecycle.

  • The specific issue is that the validation for the first bid amount uses a strict greater-than comparison (>) instead of the greater-than-or-equal-to comparison (>=) used elsewhere in the code, making the first bid requirements inconsistent and more restrictive than subsequent bids.

// For the first bid (when previousBidAmount == 0):
requiredAmount = listing.minPrice;
require(msg.value > requiredAmount, "First bid must be > min price"); @> // Strict > comparison
​
// For subsequent bids:
requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
require(msg.value >= requiredAmount, "Bid not high enough"); @> // >= comparison used here

Risk

Likelihood: Medium

  • This issue affects every auction at the first bid stage.

  • Users who attempt to bid exactly the minimum price will have their transactions unexpectedly revert.

Impact: Low

  • First-time bidders need to bid slightly higher than the minimum price even though the documentation might indicate that the minimum price is acceptable.

  • This creates a confusing user experience and might deter participation from users who don't understand why their minimum bid was rejected.

  • The financial impact is minimal as users only need to add a small amount above the minimum price (e.g., 1 wei).

Proof of Concept

import { ethers } from "hardhat";
import { expect } from "chai";
import { Contract, Signer } from "ethers";
​
describe("BidBeastsNFTMarket Incorrect Price Validation", function () {
let marketplace: Contract;
let nft: Contract;
let owner: Signer;
let alice: Signer;
let bob: Signer;
let tokenId: number;
const ONE_ETHER = ethers.utils.parseEther("1.0");
const TWO_ETHER = ethers.utils.parseEther("2.0");
beforeEach(async function () {
// Get signers
[owner, alice, bob] = await ethers.getSigners();
// Deploy NFT contract
const BidBeastsFactory = await ethers.getContractFactory("BidBeasts");
nft = await BidBeastsFactory.deploy();
await nft.deployed();
// Deploy marketplace contract
const MarketplaceFactory = await ethers.getContractFactory("BidBeastsNFTMarket");
marketplace = await MarketplaceFactory.deploy(nft.address);
await marketplace.deployed();
// Mint NFT to owner
const mintTx = await nft.mint(await owner.getAddress());
const receipt = await mintTx.wait();
tokenId = receipt.events?.find(e => e.event === "BidBeastsMinted")?.args?.tokenId;
// Approve marketplace to transfer NFT
await nft.approve(marketplace.address, tokenId);
// List NFT on marketplace
await marketplace.listNFT(tokenId, ONE_ETHER, TWO_ETHER); // minPrice = 1 ether, buyNowPrice = 2 ether
});
it("Should reject bid exactly at minimum price", async function () {
// Alice tries to bid exactly at minimum price
await expect(
marketplace.connect(alice).placeBid(tokenId, { value: ONE_ETHER })
).to.be.revertedWith("First bid must be > min price");
});
it("Should accept bid slightly above minimum price", async function () {
// Alice bids slightly above minimum price
const slightlyMore = ONE_ETHER.add(1); // 1 ether + 1 wei
await marketplace.connect(alice).placeBid(tokenId, { value: slightlyMore });
// Verify bid was accepted
const bid = await marketplace.getHighestBid(tokenId);
expect(bid.bidder).to.equal(await alice.getAddress());
expect(bid.amount).to.equal(slightlyMore);
});
it("Should accept subsequent bid at exact required amount", async function () {
// First, Alice places a bid above minimum price
const initialBid = ONE_ETHER.add(100);
await marketplace.connect(alice).placeBid(tokenId, { value: initialBid });
// Calculate exactly 5% more using the same logic as the contract
const minIncrement = 5; // S_MIN_BID_INCREMENT_PERCENTAGE
const requiredAmount = initialBid.div(100).mul(100 + minIncrement);
// Bob bids exactly the required amount
await marketplace.connect(bob).placeBid(tokenId, { value: requiredAmount });
// Verify bid was accepted
const bid = await marketplace.getHighestBid(tokenId);
expect(bid.bidder).to.equal(await bob.getAddress());
expect(bid.amount).to.equal(requiredAmount);
});
});

his proof of concept demonstrates:

  1. Inconsistent Validation: When placing the first bid exactly at the minimum price, the transaction will revert, but subsequent bids can be exactly at the minimum required increment.

  2. User Confusion: Users expecting to bid the exact minimum price will have their transactions fail without a clear understanding of why.

  3. Workaround: Users need to bid slightly above the minimum price (e.g., 1 ether + 1 wei) for their first bid to be accepted.

Recommended Mitigation

- require(msg.value > requiredAmount, "First bid must be > min price");
+ require(msg.value >= requiredAmount, "First bid must be >= min price");

This simple change:

  1. Consistent Requirements: Makes the bidding requirements consistent throughout the auction lifecycle by using the same comparison operator (>=) for both first and subsequent bids.

  2. Improved User Experience: Allows users to bid exactly the minimum price on their first bid, which matches typical user expectations.

  3. Clear Communication: Updates the error message to accurately reflect the actual requirement, avoiding confusion.

This change will make the auction system more intuitive and consistent for users without compromising the economic security of the bidding process. The consistency in validation rules will lead to fewer failed transactions and a better overall user experience.

Updates

Lead Judging Commences

cryptoghost Lead Judge 29 days ago
Submission Judgement Published
Validated
Assigned finding tags:

BidBeasts Marketplace: First Bid > Instead of >=

First bid validation uses > instead of >=, preventing valid starting bids.

Support

FAQs

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