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

Low: Percentage increment uses division before multiplication (rounding error)

andysakov

Low: Percentage increment uses division before multiplication (rounding error)

Description

  • Normal behavior: Compute required bid as previous × (100 + increment) / 100 with correct rounding down.

  • Issue: Current code divides first then multiplies, causing underestimation by truncation for many values.

155:158:2025-09-bid-beasts/src/BidBeastsNFTMarketPlace.sol

} else {
requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
require(msg.value >= requiredAmount, "Bid not high enough");
}
// @> Root: Division before multiplication loses precision

Risk

Likelihood:

  • Triggers whenever previousBidAmount % 100 != 0

  • Common in typical bidding amounts

Impact:

  • Allows slightly smaller-than-intended bids to be accepted

  • Minor revenue loss for sellers

Proof of Concept

Important detail (integer math)

Because Solidity uses integer division, the current order divides first, then multiplies:

  • Current: (prev / 100) * 105

  • Safer: (prev * 105) / 100

These are not always equal due to truncation. Example:

  • prev = 101 wei, 5%

  • Current: (101/100)=1 → 1105=105

  • Correct: (101105)/100 = 106 (truncates from 106.05)

  • Result: the current code underestimates by 1 wei, allowing a slightly smaller bid than intended.

// Foundry-style PoC: current math accepts a second bid that is 1 wei below the intended threshold.
function test_RoundingError_AllowsTooLowSecondBid() public {
// Deploy core contracts
BidBeasts nft = new BidBeasts();
BidBeastsNFTMarket market = new BidBeastsNFTMarket(address(nft));
​
// Actors
address seller = address(0xA11CE);
address b1 = address(0xB1);
address b2 = address(0xB2);
​
// Mint and list tokenId 0
vm.prank(nft.owner());
nft.mint(seller);
vm.startPrank(seller);
nft.approve(address(market), 0);
market.listNFT(0, 0.01 ether, 0);
vm.stopPrank();
​
vm.deal(b1, 1000 ether);
vm.deal(b2, 1000 ether);
​
// First bid: choose a value with a remainder when divided by 100
uint256 prev = 1 ether + 1; // 1 wei remainder
vm.prank(b1);
market.placeBid{value: prev}(0);
​
// Compute thresholds
uint256 inc = 100 + market.S_MIN_BID_INCREMENT_PERCENTAGE(); // 105
uint256 correctRequired = (prev * inc) / 100; // 1.05 ether + 1 wei
uint256 contractRequired = (prev / 100) * inc; // 1.05 ether (1 wei less)
assertLt(contractRequired, correctRequired, "No rounding gap");
​
// Second bid equals contractRequired should be accepted by current code
vm.prank(b2);
market.placeBid{value: contractRequired}(0);
​
// But that bid is still below the intended correctRequired threshold by 1 wei
assertLt(contractRequired, correctRequired, "Second bid should be too low under correct math");
​
// Verify state reflects acceptance of the under-threshold bid
BidBeastsNFTMarket.Bid memory highest = market.getHighestBid(0);
assertEq(highest.bidder, b2);
assertEq(highest.amount, contractRequired);
}

Recommended Mitigation

This method preserves more precision before the final truncation.

- requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
+ 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!