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

[M-1] - `BidBeastsNFTMarket::placeBid` Divide before multiply cause precision loss for the `requiredAmount` calculation.

thecil

Root + Impact

[M-1] - BidBeastsNFTMarket::placeBid Divide before multiply cause precision loss for the requiredAmount calculation.

Description

Solidity's integer division truncates. Thus, performing division before multiplication can lead to precision loss.

The function placeBid calculates the requiredAmount for a bid based on the previous bid amount and a minimum increment percentage. The division operation is performed before multiplication, which can lead to precision loss.

This is the actual code base for the placeBid function in the BidBeastsNFTMarket contract which is triggered on the --- Regular Bidding Logic --- section of the code.

function placeBid(uint256 tokenId) external payable isListed(tokenId) {
...
// --- Regular Bidding Logic ---
uint256 requiredAmount;
if (previousBidAmount == 0) {
...
} else
@> requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
}

Risk

Likelihood: Medium.

  • Reason : Every time a bid is placed, there is a risk of precision loss due to the division operation before multiplication.

Impact: Medium.

Proof of Concept

(Proof of Code)

  1. In the BidBeastsMkartePlaceTest.t.sol unit test file, place the following unit test:

function _placeBid(address user, uint256 tokenId, uint256 amount) private {
vm.startPrank(user);
market.placeBid{value: amount}(tokenId);
vm.stopPrank();
}
​
function test_requiredAmount() public {
// setup context
_mintNFT();
// list the nft with minimum bid price
vm.startPrank(SELLER);
nft.approve(address(market), TOKEN_ID);
market.listNFT(TOKEN_ID, 0.01 ether, BUY_NOW_PRICE);
vm.stopPrank();
// place a bid with min bid price plus 1 wei, so we can prove how the precision for that 1 wei is lost
_placeBid(BIDDER_1, TOKEN_ID, 0.01 ether + 1);
BidBeastsNFTMarket.Bid memory highestBid = market.getHighestBid(
TOKEN_ID
);
uint256 markeBidtIncrement = market.S_MIN_BID_INCREMENT_PERCENTAGE();
uint256 prevBidAmount = highestBid.amount;
​
// required amount actual code base calculation
uint256 requiredAmount = (prevBidAmount / 100) *
(100 + markeBidtIncrement);
​
// correct formula
uint256 correctRequiredAmountformula = (prevBidAmount *
(100 + markeBidtIncrement)) / 100;
​
console.log("prevBidAmount: %e", prevBidAmount);
console.log("requiredAmount: %e", requiredAmount);
console.log(
"correctRequiredAmountformula: %e",
correctRequiredAmountformula
);
}

  1. Run the unit test to demostrate the exploit.

forge test --mt test_requiredAmount -vv

By comparing the result of the actual requiredAmount formula against the correctRequiredAmountformula we can realize a round-down error because of integer division before multiplication.

Recommended Mitigation

Use the formula proposed at the unit test as correctRequiredAmountformula

function placeBid(uint256 tokenId) external payable isListed(tokenId) {
...
// --- Regular Bidding Logic ---
uint256 requiredAmount;
if (previousBidAmount == 0) {
...
} else
- 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 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!