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 25 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.