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 3 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!