Bid Beasts
First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: low
Invalid

Possible Reentrancy vulnerability in listNFT Function

the2ke

State updated after transfer

Description

  • The `listNFT()` performs an external call `transferFrom` before updating the contract state, this violate the CEI(Checks-Effects-Interactions) pattern. This creates a possible reentrancy vulnerability where a malicious NFT could re-enter the function before the listing state is updated.Explain the specific issue or problem in one or more sentences

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"
);
}
​
@> BBERC721.transferFrom(msg.sender, address(this), tokenId);
​
listings[tokenId] = Listing({
seller: msg.sender,
minPrice: _minPrice,
buyNowPrice: _buyNowPrice,
auctionEnd: 0, // Timer starts only after the first valid bid.
listed: true
});
​
emit NftListed(tokenId, msg.sender, _minPrice, _buyNowPrice);
}

Risk

Likelihood:

  • This occurs if the msg.sender is a contract

  • The reentrancy occurs during the NFT transfer's callback, before the listing state is updated, allowing duplicate listings.


Impact:

  • During NFT transfer, malicious contract can call `listNft()` again

  • THis can even possibly block NFT from being listed properlyProof of Concept


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"
);
}
- BBERC721.transferFrom(msg.sender, address(this), tokenId);
listings[tokenId] = Listing({
seller: msg.sender,
minPrice: _minPrice,
buyNowPrice: _buyNowPrice,
auctionEnd: 0, // Timer starts only after the first valid bid.
listed: true
});
+ BBERC721.transferFrom(msg.sender, address(this), tokenId);
emit NftListed(tokenId, msg.sender, _minPrice, _buyNowPrice);
}
Updates

Lead Judging Commences

cryptoghost Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

the2ke Submitter
about 1 month ago
cryptoghost Lead Judge
about 1 month ago
cryptoghost Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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