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

[M-3] Precision loss in `BidBeastsNFTMarket::placeBid` function calculation of `requiredAmount` for next bid, possibly causing denial of service.

alexscherbatyuk

Precision loss in BidBeastsNFTMarket::placeBid function calculation of requiredAmount for next bid, possibly causing denial of service.

Description

  • The current implementation of the `requiredAmount` formula calculation loses precision for some bid amounts, causing some valid bids to revert.

  • Denial of service for users.

function placeBid(uint256 tokenId) external payable isListed(tokenId) {
...
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 {
//@> 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) {
listing.auctionEnd = listing.auctionEnd + S_AUCTION_EXTENSION_DURATION;
emit AuctionExtended(tokenId, listing.auctionEnd);
}
}
​
// EFFECT: update highest bid
bids[tokenId] = Bid(msg.sender, msg.value);
​
if (previousBidder != address(0)) {
_payout(previousBidder, previousBidAmount);
}
​
emit BidPlaced(tokenId, msg.sender, msg.value);
}

Risk

Likelihood: Medium

  • When bid amounts don't end in "00" (not divisible by 100)

  • It might occur under specific conditions.

Impact: Medium

  • Denial of service for users with valid bid

  • Some level of disruption to the protocol's functionality.

Proof of Concept

Add the following code snippet to the `BidBeastsMarketPlaceTest.t.sol` test file.

function testRequiredAmountCalculation(uint256 bidAmount) public pure {
bidAmount = bound(bidAmount, 0.01 ether, 1 ether);
uint256 S_MIN_BID_INCREMENT_PERCENTAGE = 5;
uint256 previousBidAmount = bidAmount;
uint256 requiredAmountCorrect = previousBidAmount + (previousBidAmount * S_MIN_BID_INCREMENT_PERCENTAGE) / 100;
uint256 requiredAmountInCorrect = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
​
uint256 precisionDifference = requiredAmountCorrect - requiredAmountInCorrect;
​
console.log("Precision difference: ", precisionDifference);
​
assertEq(precisionDifference, 0, "Precision difference is greater than 0");
}

Recommended Mitigation

Modify the `BidBeastsNFTMarket::placeBid` function to calculate the `requiredAmount` correctly.

function placeBid(uint256 tokenId) external payable isListed(tokenId) {
...
​
- requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
+ requiredAmount = previousBidAmount + (previousBidAmount * 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!