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

[L-2] Unsafe ERC20 transferFrom — Not Using SafeERC20

Author Revealed upon completion

Root + Impact

Description

  • Some ERC20 tokens do not return a bool from transferFrom (e.g. USDT on mainnet). The contract already imports and uses SafeERC20 for some calls but falls back to raw transferFrom with a require(success, ...) check in two places. If the USDC token used were non-standard, these calls would revert incorrectly or silently fail.

// src/NFTDealers.sol:119
@> require(usdc.transferFrom(msg.sender, address(this), lockAmount), "USDC transfer failed");
​
// src/NFTDealers.sol:148
@> bool success = usdc.transferFrom(msg.sender, address(this), listing.price);
require(success, "USDC transfer failed");

Risk

Likelihood:

  • MockUSDC and standard USDC both return bool correctly.

  • Risk materialises if the contract is redeployed with a non-standard token.

Impact:

  • Transaction reverts unexpectedly on non-standard tokens; no fund loss with standard USDC.

Proof of Concept

// With a non-returning token like USDT:
// usdc.transferFrom(...) returns nothing → require(success) receives default false → reverts
// The buyer's USDC is not taken but the transaction also fails silently

Recommended Mitigation

Replace raw transferFrom calls with safeTransferFrom from SafeERC20 to handle non-standard tokens uniformly.

- require(usdc.transferFrom(msg.sender, address(this), lockAmount), "USDC transfer failed");
+ usdc.safeTransferFrom(msg.sender, address(this), lockAmount);
​
- bool success = usdc.transferFrom(msg.sender, address(this), listing.price);
- require(success, "USDC transfer failed");
+ usdc.safeTransferFrom(msg.sender, address(this), listing.price);

Support

FAQs

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

Give us feedback!