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

In `BidBeastNFTMarletPlace:placeBid`, Arithmetic precision loss due to operator precedence subsequent bid requiredAmount

vincent71399

Description: The new bid requiredAmount is calculated base on the previous bid amount plus a percentage increment. however, the calculation does division before multiplication, leading to potential precision loss due to integer division truncation.

function placeBid(uint256 tokenId) external payable isListed(tokenId) {
...
@> requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
}

Impact: This precision loss can lead to scenarios when the initial bid is low, resulting in a lower than expected requiredAmount for subsequent bids.

Proof of Concept: Add the following test to BidBeastMarketPlaceTest.t.sol and run testSubsequentBidsRequireLowerThanInitBid

function testSubsequentBidsRequireLowerThanInitBid() public {
_mintNFT();
uint256 initBid = market.S_MIN_NFT_PRICE() + 98;
// list with low min price
vm.startPrank(SELLER);
nft.approve(address(market), TOKEN_ID);
market.listNFT(TOKEN_ID, initBid, BUY_NOW_PRICE);
vm.stopPrank();
// first bid
vm.prank(BIDDER_1);
market.placeBid{value: initBid + 1}(TOKEN_ID);
// second bid lower than min increment
uint256 secondBidAmount = market.S_MIN_NFT_PRICE() * (100 + market.S_MIN_BID_INCREMENT_PERCENTAGE()) / 100;
vm.prank(BIDDER_2);
market.placeBid{value: secondBidAmount}(TOKEN_ID); // a small portion of the initBid is neglected due to rounding
assertEq(market.getHighestBid(TOKEN_ID).bidder, BIDDER_2);
assertEq(market.getHighestBid(TOKEN_ID).amount, secondBidAmount);
}

Recommended Mitigation:
reorder the calculation in BidBeastNFTMarletPlace:placeBid to perform multiplication before division to maintain precision.

function placeBid(uint256 tokenId) external payable isListed(tokenId) {
...
- 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 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.