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 about 1 month 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.