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

NFT Re-Listing Vulnerability: Market Contract Can Overwrite Seller and Lock Assets

ilhamitugral

Root + Impact

Description

  • Normal behavior: The BidBeastsNFTMarket::listNFT function is intended to allow the current human NFT owner to list their token on the marketplace. After a successful listing, the NFT is transferred into escrow (address(market)) and the seller is recorded in the listings mapping so that only they can unlist and receive proceeds.

  • Issue: After the first listing, BidBeasts::ownerOf(tokenId) becomes the marketplace contract. Since the code checks only require(BBERC721.ownerOf(tokenId) == msg.sender), if msg.sender is also the marketplace, the contract can call its own BidBeastsNFTMarket::listNFT function. This overwrites the seller field in the listing with address(this). As a result, the real seller loses the ability to unlist, and any sales will send funds to the contract itself with no withdrawal path.

function listNFT(uint256 tokenId, uint256 _minPrice, uint256 _buyNowPrice) external {
// @> owner check passes when msg.sender == address(this) after first listing
require(BBERC721.ownerOf(tokenId) == msg.sender, "Not the owner");
​
BBERC721.transferFrom(msg.sender, address(this), tokenId);
​
// @> seller overwritten by msg.sender (market contract itself)
listings[tokenId] = Listing({
seller: msg.sender,
minPrice: _minPrice,
buyNowPrice: _buyNowPrice,
auctionEnd: 0,
listed: true
});
​
emit NftListed(tokenId, msg.sender, _minPrice, _buyNowPrice);
}

Risk

Likelihood:

  • Once a token is listed, its BidBeasts::ownerOf changes to the marketplace contract, so any internal context where msg.sender == address(market) can re-trigger BidBeastsNFTMarket::listNFT.

  • No additional check (!listed) prevents overwriting, making this scenario consistently reproducible.

Impact:

  • Loss of funds: On sale, the seller is the marketplace itself. Proceeds are sent to the contract, not the real seller, and remain locked.

  • Permanent lock: The original seller cannot call BidBeastsNFTMarket::unlistNFT anymore because BidBeastsNFTMarket::isSeller check fails. This prevents reclaiming their NFT.

Proof of Concept

function test_ListNFT_RelistByMarket_SellerHijack() public {
​
uint256 tokenId = 0;
​
// 1) Mint NFT's to SELLER
vm.startPrank(OWNER);
nft.mint(SELLER);
vm.stopPrank();
assertEq(nft.ownerOf(tokenId), SELLER);
​
// 2) List NFT as a SELLER
vm.startPrank(SELLER);
nft.setApprovalForAll(address(market), true);
market.listNFT(tokenId, 1 ether, 0);
vm.stopPrank();
assertEq(nft.ownerOf(tokenId), address(market));
assertTrue(market.getListing(tokenId).listed);
assertEq(market.getListing(tokenId).seller, SELLER);
​
// 3) Let's list NFT again as Contract Address
vm.startPrank(address(market));
market.listNFT(tokenId, 1.5 ether, 0);
vm.stopPrank();
assertTrue(market.getListing(tokenId).listed);
assertEq(market.getListing(tokenId).seller, address(market)); // SELLER is not longer seller
​
// 4) Original SELLER wants cancel listing NFT
// We expecting revert now.
vm.startPrank(SELLER);
vm.expectRevert();
market.unlistNFT(tokenId); // REVERTED (Current Owner => address(market))
vm.stopPrank();
}

Result:

Ran 1 test for test/BidBeastsMarketPlaceTest.t.sol:BidBeastsNFTMarketTest
[PASS] test_ListNFT_RelistByMarket_SellerHijack() (gas: 261986)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.27ms (494.70µs CPU time)
​
Ran 1 test suite in 35.87ms (4.27ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Explanation:
This test shows that after the NFT is escrowed, the marketplace can re-list it as if it were the seller. The overwrite breaks accounting and leaves both the funds and NFT effectively stuck.

Recommended Mitigation

function listNFT(uint256 tokenId, uint256 _minPrice, uint256 _buyNowPrice) external {
+ require(!listings[tokenId].listed, "Already listed");
+ require(msg.sender != address(this), "Market cannot list its own NFTs");

Explanation:

  1. Add a guard to prevent re-listing (!listings[tokenId].listed).

  2. Explicitly forbid the marketplace itself from being recorded as the seller (msg.sender != address(this)).

  3. Consider storing the original seller in an immutable field for the listing lifetime and never overwriting it.

  4. Optionally, implement a withdrawal fallback only for user funds — but ideally this should be unnecessary if the invariant is preserved.

Updates

Lead Judging Commences

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.