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 28 days 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.