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

M01. Bid increment calculation (integer division bug)

accessdenied

Root + Impact

Description

  • Normal behavior: Each new bid must increase the previous highest bid by at least the configured minimum increment percentage (here S_MIN_BID_INCREMENT_PERCENTAGE = 5). The expected calculation is “new required = previous * (100 + increment) / 100” so that bids always increase by at least 5%.

  • Issue: The implementation computes the required next bid as (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE). Because Solidity does integer division that truncates toward zero, performing division before multiplication loses precision. That truncation results in a required bid that can be smaller than the mathematically correct required amount. An adversary can place a bid that is lower than the intended minimum increment and still be accepted.

// Root cause in the codebase with @> marks to highlight the relevant section
// code fragment from placeBid(...)
uint256 requiredAmount;
​
if (previousBidAmount == 0) {
requiredAmount = listing.minPrice;
require(msg.value > requiredAmount, "First bid must be > min price");
} else {
// BUG: division happens before multiplication causing truncation loss
@> requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
require(msg.value >= requiredAmount, "Bid not high enough");
}
​

Risk

Likelihood:

  • Many real bids are not exact multiples of 100; as soon as previousBidAmount is not divisible by 100 the expression previousBidAmount / 100 truncates fractional wei portions. This truncation always occurs whenever previousBidAmount % 100 != 0.

  • Auction minimums (e.g., seller minPrice) do not eliminate this case — typical min prices (even 0.01 ether or 1 ether) can produce non-divisible numbers after user-added wei (user may bid with arbitrary wei granularity).

Impact:

  • Bidders can outbid the previous bid with an amount smaller than the intended 5% increment and still become highest bidder. This lets attackers slightly undercut required increments and save ETH on each outbid.

  • Over many auctions or high volume bidding this allows marginal economic advantage and undermines the integrity of the minimum increment rule. It also complicates off-chain analytics that assume the increment is enforced exactly.

  • In extreme, if other controls were smaller (e.g., if S_MIN_NFT_PRICE could be very small), the same bug could allow accepting trivially small bids; here it’s a moderate degrading of enforcement .

Proof of Concept

The following Foundry-style test demonstrates the issue. It places a first bid with a value that is not divisible by 100 (previousBid = MIN_PRICE + 1 wei), computes the required next bid according to the contract's buggy formula, and shows the contract accepts that smaller-than-mathematically-correct bid.

function test_poc_bidIncrementIntegerDivision() public {
// Setup environment: mint and list
vm.startPrank(OWNER);
nft.mint(SELLER); // tokenId 0
vm.stopPrank();
​
vm.startPrank(SELLER);
nft.approve(address(market), TOKEN_ID);
// MIN_PRICE (in these tests) = 1 ether
market.listNFT(TOKEN_ID, MIN_PRICE, BUY_NOW_PRICE);
vm.stopPrank();
​
// 1) Place first bid that is NOT divisible by 100: MIN_PRICE + 1 wei
uint256 firstBid = MIN_PRICE + 1;
vm.prank(BIDDER_1);
market.placeBid{value: firstBid}(TOKEN_ID);
​
// 2) Compute the required next bid *as the contract does it* (buggy)
uint256 previous = market.getHighestBid(TOKEN_ID).amount; // previous == firstBid
uint256 buggyRequired = (previous / 100) * (100 + market.S_MIN_BID_INCREMENT_PERCENTAGE());
​
// 3) Compute the mathematically correct required amount for comparison
uint256 correctRequired = (previous * (100 + market.S_MIN_BID_INCREMENT_PERCENTAGE())) / 100;
​
// sanity checks: buggyRequired is <= correctRequired, and for this chosen previous
assertTrue(buggyRequired < correctRequired, "Bug not triggered with these values");
​
// 4) Bidder_2 bids exactly buggyRequired (which is less than the true required)
vm.prank(BIDDER_2);
market.placeBid{value: buggyRequired}(TOKEN_ID);
​
// The contract accepts the bid even though it is smaller than the mathematically correct increment.
(address bidderAddr, uint256 amount) = (
market.getHighestBid(TOKEN_ID).bidder,
market.getHighestBid(TOKEN_ID).amount
);
assertEq(bidderAddr, BIDDER_2, "Buggy bid was not accepted");
assertEq(amount, buggyRequired, "Highest bid amount not equal to buggy computed amount");
​
// Demonstrate numeric gap: difference > 0
uint256 delta = correctRequired - buggyRequired;
assertTrue(delta > 0, "There should be a non-zero shortfall due to truncation");
}
​

Result: The marketplace accepts buggyRequired even though the correct required amount is correctRequired (> buggyRequired). The difference delta is the amount by which the enforced minimum is violated (often small — a few wei).

Recommended Mitigation

  • Compute the required next bid by multiplying first, then dividing to avoid integer truncation loss.

  • Optionally, to be stricter and always round up so the bidder must exceed the exact percentage (i.e., no rounding benefit to bidder), compute with rounding-up for the increment.

Minimal safe fix (multiply first):

- requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
+ // Safer: multiply first, then divide to avoid truncation loss.
+ requiredAmount = (previousBidAmount * (100 + S_MIN_BID_INCREMENT_PERCENTAGE)) / 100;
​
Updates

Lead Judging Commences

cryptoghost Lead Judge 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

BidBeasts Marketplace: Integer Division Precision Loss

Integer division in requiredAmount truncates fractions, allowing bids slightly lower than intended.

Support

FAQs

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

Give us feedback!