Bid Beasts

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

Operation associated with multiplication and division should always start with multiplication

Multiplication performs on a result of division had a possibility of creating weird integers

Description

Since Solidity only deals with whole numbers. floating numbers usually got rounded up or down

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);
}
}
...
}

Risk

Likelihood: There is a possibily of an attacker manipulating the `requiredAmount` into being less than what it's need to be.

Impact:

Manipulation of `requiredAmount` may impact price that the attacker pay become less than it should.

Proof of Concept

function testProofMultiplicationFirstThanDivisionIsBetter () public pure{
uint256 number = 10;
uint256 increasePercentage = 10;
uint256 correctNumberToAdd = (number * increasePercentage) / 100;
uint256 correctNumber = number + correctNumberToAdd;
uint256 expectedCorrectNumber = 11;
assertEq(correctNumber, expectedCorrectNumber);
uint256 wrongNumberToAdd = (number / 100) * 10;
uint256 wrongNumber = number + wrongNumberToAdd;
assertEq(wrongNumber, expectedCorrectNumber);
}

Recommended Mitigation

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 = (previousBidAmoun * (100 + S_MIN_BID_INCREMENT_PERCENTAGE)) / 100;
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);
}
}
...
}
```
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!