NFT Dealers
First Flight #58
Beginner FriendlyFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: low
Likelihood: low

L06. `buy()` uses raw `transferFrom` with `require` instead of `safeTransferFrom`, inconsistent with SafeERC20 used elsewhere

Author Revealed upon completion

Root + Impact

Description

  • The contract declares using SafeERC20 for IERC20 and uses usdc.safeTransfer or usdc.safeTransferFrom in most USDC movements.

  • buy() calls usdc.transferFrom directly and wraps it in a require on the return value, the same anti-pattern already present in mintNft(). For ERC20 tokens that return no boolean on transferFrom (non-compliant, void-returning tokens), the require causes Solidity 0.8 to attempt ABI-decoding empty returndata as bool, triggering a decoding revert rather than the intended "USDC transfer failed" message.

function buy(uint256 _listingId) external payable {
Listing memory listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId);
require(listing.seller != msg.sender, "Seller cannot buy their own NFT");
​
activeListingsCounter--;
​
// @> raw transferFrom with require — inconsistent with SafeERC20 used elsewhere
bool success = usdc.transferFrom(msg.sender, address(this), listing.price);
require(success, "USDC transfer failed");
​
_safeTransfer(listing.seller, msg.sender, listing.tokenId, "");
s_listings[_listingId].isActive = false;
emit NFT_Dealers_Sold(msg.sender, listing.price);
}

Risk

Likelihood: Low

  • Standard USDC always returns true on a successful transferFrom, so this causes no practical issue with the intended deployment token.

  • The risk only materialises if a non-standard ERC20 (returning void instead of bool) is used as the payment token at deploy time.

Impact: Low

  • With standard USDC the behavior is identical to safeTransferFrom.

  • With a void-returning token, Solidity 0.8 attempts to ABI-decode empty returndata as bool, reverting with a decoding error rather than "USDC transfer failed", making the failure harder to diagnose.

  • Two instances of this pattern exist in the contract (mintNft and buy), increasing code inconsistency and audit surface.

Proof of Concept

The inconsistency is visible by comparing buy() with the SafeERC20 calls in other functions:

// In buy() — raw call with require:
bool success = usdc.transferFrom(msg.sender, address(this), listing.price);
require(success, "USDC transfer failed");
​
// In mintNft() — also raw call with require (L-05):
require(usdc.transferFrom(msg.sender, address(this), lockAmount), "USDC transfer failed");
​
// In cancelListing(), collectUsdcFromSelling(), withdrawFees() — SafeERC20:
usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]);
usdc.safeTransfer(msg.sender, amountToSeller);
usdc.safeTransfer(owner, amount);

For a token that returns void on transferFrom, Solidity 0.8 tries to ABI-decode the empty returndata as bool. This triggers a decoding revert rather than the intended "USDC transfer failed" string. SafeERC20.safeTransferFrom handles this correctly by checking returndata.length == 0 || abi.decode(returndata, (bool)).

Recommended Mitigation

Replace the raw require(usdc.transferFrom(...)) pattern in buy() with usdc.safeTransferFrom(), consistent with SafeERC20 usage throughout the rest of the contract. Apply the same fix to mintNft() (L-05).

function buy(uint256 _listingId) external payable {
Listing memory listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId);
require(listing.seller != msg.sender, "Seller cannot buy their own NFT");
​
activeListingsCounter--;
​
- bool success = usdc.transferFrom(msg.sender, address(this), listing.price);
- require(success, "USDC transfer failed");
+ usdc.safeTransferFrom(msg.sender, address(this), listing.price);
​
_safeTransfer(listing.seller, msg.sender, listing.tokenId, "");
s_listings[_listingId].isActive = false;
emit NFT_Dealers_Sold(msg.sender, listing.price);
}

Support

FAQs

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

Give us feedback!