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

Integer Division Precision Loss in Bid Increment Calculation Allows Unfair Bidding

vk1033

Integer Division Precision Loss in Bid Increment Calculation

Description

  • The normal behavior should be that new bids must be at least 5% higher than the previous bid to ensure fair auction progression and prevent spam bidding.

  • The current implementation uses integer division before multiplication, causing precision loss that allows bidders to place bids lower than the intended 5% increment, giving them an unfair advantage.

function placeBid(uint256 tokenId) external payable isListed(tokenId) {
// --- Buy Now Logic ---
// ...
​
// --- Regular Bidding Logic ---
uint256 requiredAmount;
if (previousBidAmount == 0) {
requiredAmount = listing.minPrice;
require(msg.value > requiredAmount, "First bid must be > min price");
listing.auctionEnd = block.timestamp + S_AUCTION_EXTENSION_DURATION;
emit AuctionExtended(tokenId, listing.auctionEnd);
} else {
//q, is this cooroect?
@> requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
require(msg.value >= requiredAmount, "Bid not high enough");
​
uint256 timeLeft = 0;
if (listing.auctionEnd > block.timestamp) {
timeLeft = listing.auctionEnd - block.timestamp;
}
if (timeLeft < S_AUCTION_EXTENSION_DURATION) {
//a, there is not code to check for 3 days deadline
//a, for it to be exactly S_AUCTION_EXTENSION_DURATION,
// listing.auctionEnd = listing.auctionEnd + S_AUCTION_EXTENSION_DURATION - timeLeft;
listing.auctionEnd = listing.auctionEnd + S_AUCTION_EXTENSION_DURATION;
emit AuctionExtended(tokenId, listing.auctionEnd);
}
}
// EFFECT: update highest bid
//...
}

Risk

Likelihood: High

  • The vulnerability occurs on every bid after the first one

  • Affects all auctions with bid amounts that aren't perfect multiples of 100

  • Precision loss becomes more significant with smaller bid amounts

Impact: Medium

  • Unfair bidding advantage: Bidders can place significantly lower bids than intended

  • Bidding rules don't work as designed

Proof of Concept

The PoC demonstrates how the integer division precision loss allows bids that are significantly lower than the required 5% increment.

Add the test below to the BidBeastsMarketPlaceTest.t.sol:

function testBidIncrementMathError() public {
_mintNFT();
_listNFT();
​
// Place first bid with an amount that will cause precision loss
uint256 FIRST_BID = MIN_PRICE + 199; // Amount that causes precision loss when divided by 100
vm.prank(BIDDER_1);
market.placeBid{value: FIRST_BID}(TOKEN_ID);
​
// Calculate what the increment should be vs what it actually is
uint256 S_MIN_BID_INCREMENT_PERCENTAGE = 5;
​
// Incorrect calculation (what the contract does)
uint256 calculateWrong = (FIRST_BID / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
​
// Correct calculation (what it should be)
uint256 calculateCorrect = (FIRST_BID * (100 + S_MIN_BID_INCREMENT_PERCENTAGE)) / 100;
​
console.log("Previous bid:", FIRST_BID); // 1000000000000000199
console.log("Wrong calculation result:", calculateWrong); // 1050000000000000105
console.log("Correct calculation result:", calculateCorrect); // 1050000000000000208
console.log("Precision loss:", calculateCorrect - calculateWrong); // 103
​
// Verify the math error exists
assertTrue(calculateWrong < calculateCorrect, "Contract calculation should be less than correct calculation");
​
// Show that a bid much lower than 5% increment is accepted
vm.prank(BIDDER_2);
market.placeBid{value: calculateWrong}(TOKEN_ID);
​
// Verify the low bid was accepted
assertEq(market.getHighestBid(TOKEN_ID).amount, calculateWrong, "Low bid should be accepted due to math error");
assertEq(market.getHighestBid(TOKEN_ID).bidder, BIDDER_2, "BIDDER_2 should be highest bidder");
}

Run the test with:

forge test --match-path test/BidBeastsMarketPlaceTest.t.sol --match-test testBidIncrementMathError

Recommended Mitigation

Fix the order of operations to perform multiplication before division, preventing precision loss:

function placeBid(uint256 tokenId) external payable isListed(tokenId) {
// ... existing code ...
} else {
- requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
+ requiredAmount = (previousBidAmount * (100 + S_MIN_BID_INCREMENT_PERCENTAGE)) / 100;
require(msg.value >= requiredAmount, "Bid not high enough");
// ... existing code ...
}
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!