Submission Details
Impact:
Likelihood:
Reentrancy: State Change After External Call
Description
-
Smart contracts should follow the Checks-Effects-Interactions (CEI) pattern to prevent reentrancy attacks.
-
The
BidBeastsNFTMarketPlace::listNFT()function changes contract state after making external calls, creating potential reentrancy vulnerabilities where malicious contracts could manipulate state during external calls.
BBERC721.transferFrom(msg.sender, address(this), tokenId);
@> listings[tokenId] = Listing({
seller: msg.sender,
minPrice: _minPrice,
buyNowPrice: _buyNowPrice,
auctionEnd: 0,
listed: true
});
Risk
Likelihood:
-
Occurs when users interact with the contract using smart contract wallets
-
Malicious NFT contracts could exploit this pattern
Impact:
-
Potential state manipulation during external calls
-
Inconsistent contract state if reentrancy occurs
Proof of Concept
function test_HIGH_H2_ReentrancyInListNFT() public {
vm.prank(OWNER);
uint256 tokenId = nft.mint(address(attacker));
// Setup attacker
vm.prank(address(attacker));
nft.approve(address(market), tokenId);
attacker.startAttack(tokenId);
// VULNERABILITY: listNFT changes state after external calls
// First external call: BBERC721.ownerOf(tokenId) == msg.sender
// Then: BBERC721.transferFrom(msg.sender, address(this), tokenId)
// Finally: State change with listings[tokenId] = Listing(...)
vm.prank(address(attacker));
market.listNFT(tokenId, MIN_PRICE, BUY_NOW_PRICE);
// Check if reentrancy was attempted
assertGt(attacker.attackCount(), 0, "Reentrancy should have been attempted");
// Listing should still be created (reentrancy doesn't break this particular case)
assertTrue(market.getListing(tokenId).listed, "NFT should be listed");
}
Recommended Mitigation
function listNFT(uint256 tokenId, uint256 _minPrice, uint256 _buyNowPrice) external {
require(BBERC721.ownerOf(tokenId) == msg.sender, "Not the owner");
require(_minPrice >= S_MIN_NFT_PRICE, "Min price too low");
if (_buyNowPrice > 0) {
require(_minPrice <= _buyNowPrice, "Min price cannot exceed buy now price");
}
+ // EFFECTS: Change state before external interactions
+ listings[tokenId] = Listing({
+ seller: msg.sender,
+ minPrice: _minPrice,
+ buyNowPrice: _buyNowPrice,
+ auctionEnd: 0,
+ listed: true
+ });
- BBERC721.transferFrom(msg.sender, address(this), tokenId);
-
- listings[tokenId] = Listing({
- seller: msg.sender,
- minPrice: _minPrice,
- buyNowPrice: _buyNowPrice,
- auctionEnd: 0,
- listed: true
- });
+ // INTERACTIONS: External calls after state changes
+ BBERC721.transferFrom(msg.sender, address(this), tokenId);
emit NftListed(tokenId, msg.sender, _minPrice, _buyNowPrice);
}
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.