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 about 1 month 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.