Submission Details
Severity:
Bad Practice: CEI pattern violated and Division before Mulipication
Bad Practice: CEI pattern violated and Division before Mulipication
Description
CEI pattern violated: in
BidBeastNFTMarket::_executeSale()function, no callback occured because protocol usingtransferFrommethod.Div before Mul: in
BidBeastsNFTMarket::placeBidfunction fornextBidderrequired amount, no precision loss occoured. But, can confuse user/reviewer who see it for the first time.
In
BidBeastsNFTMarket::_executeSale():
/**
* @notice Internal function to handle the final NFT transfer and payment distribution.
*/
function _executeSale(uint256 tokenId) internal {
Listing storage listing = listings[tokenId];
Bid memory bid = bids[tokenId];
listing.listed = false;
delete bids[tokenId];
//@audit-issue CEI pattern violated no reentrancy
@> BBERC721.transferFrom(address(this), bid.bidder, tokenId);
uint256 fee = (bid.amount * S_FEE_PERCENTAGE) / 100;
s_totalFee += fee;
uint256 sellerProceeds = bid.amount - fee;
_payout(listing.seller, sellerProceeds);
emit AuctionSettled(tokenId, bid.bidder, listing.seller, bid.amount);
}
In
BidBeastsNFTMarket::placeBid():
function placeBid(uint256 tokenId) external payable isListed(tokenId) {
....
....
....
} else {
//@audit-issue low, bad practice division before multipication
// can confuse user/reviewer who see it for the first time.
@> 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);
}
}
....
}
Recommended Mitigation
for
BidBeastsNFTMarket::_executeSale():
/**
* @notice Internal function to handle the final NFT transfer and payment distribution.
*/
function _executeSale(uint256 tokenId) internal {
Listing storage listing = listings[tokenId];
Bid memory bid = bids[tokenId];
listing.listed = false;
delete bids[tokenId];
- BBERC721.transferFrom(address(this), bid.bidder, tokenId);
uint256 fee = (bid.amount * S_FEE_PERCENTAGE) / 100;
s_totalFee += fee;
uint256 sellerProceeds = bid.amount - fee;
_payout(listing.seller, sellerProceeds); // pay the seller first
+ BBERC721.transferFrom(address(this), bid.bidder, tokenId);
emit AuctionSettled(tokenId, bid.bidder, listing.seller, bid.amount);
}
* for BidBeastsNFTMarket::placeBid() :
function placeBid(uint256 tokenId) external payable isListed(tokenId) {
....
....
....
} else {
- requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);
+ requiredAmount = (previousBidAmount * (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 about 1 month ago
Submission Judgement Published Assigned finding tags:
BidBeasts Marketplace: Integer Division Precision Loss
Integer division in requiredAmount truncates fractions, allowing bids slightly lower than intended.
Rewards breakdown
nSLOC
170This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.