Bid Beasts

First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Impact: medium
Likelihood: low
Invalid

Missing checks in the `BidBeastsNFTMarket::withdrawAllFailedCredits` function

Incorrect Math in BidBeastsNFTMarket::placeBid function causing precision loss in some cases.

Description: In the BidBeastsNFTMarket::placeBid function, while calculating the requiredAmount value, math is not correct as it is reccomended to multiply first before dividing , which is not followed here .

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; // @ audit there is no need of checking this again I think ,because it was already checked above
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);
}
}
...
}

Likelihood Low

  • Most of the NFT's will be in the `ether` form and also some good values rather than random long values, which means it won't cause much difference in a normal scenario.

Impact: Medium

  • The incorrect math here can lead to precision loss in the requiredAmount

Proof of Concept:

Following code will show the Incorrect Math problem.

function test_precisionCanLoseDueToIncorrectMath() public {
_mintNFT();
_listNFT();
uint256 bidAmount = 2222222222222222222; //2.222222222222222222e18
vm.prank(BIDDER_1);
market.placeBid{value: bidAmount}(TOKEN_ID);
uint256 nextMinBidAmountExpected = (bidAmount * (105)) / 100; //2333333333333333333.1 or 2.3333333333333333331e19
uint256 nextMinBidAmountReal = (bidAmount / 100) * 105;
vm.prank(BIDDER_2);
market.placeBid{value: nextMinBidAmountReal}(TOKEN_ID);
assert(nextMinBidAmountExpected > nextMinBidAmountReal);
}

Recommended Mitigation: It is recommended to multiply all the values first and then divide.

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);
+ requiredAmount=(previousBidAmount*(100 + S_MIN_BID_INCREMENT_PERCENTAGE))/100;
...
}
...
}
Updates

Lead Judging Commences

cryptoghost Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.