HardhatDeFi
15,000 USDC
View results
Submission Details
Severity: low
Invalid

Unchecked ERC20 transferFrom Return Values Result in Silent Transfer Failures and Contract State Inconsistencies

Summary

The identified code segments fail to validate the return value of the transferFrom function, which violates the ERC20 token standard security practices. Missing return value checks could allow failed token transfers to go undetected, leading to inconsistencies in contract state and potential loss of user funds.

Vulnerability Details

  1. AaveDIVAWrapperCore.sol Lines 229-230:

    _shortTokenContract.transferFrom(msg.sender, address(this), _positionTokenAmountToRemove);
    _longTokenContract.transferFrom(msg.sender, address(this), _positionTokenAmountToRemove);
  2. AaveDIVAWrapperCore.sol Lines 285-289:

    _positionTokenContract.transferFrom(
    msg.sender /** from */,
    address(this) /** to */,
    _positionTokenAmountToRedeem
    );

Issue:
The ERC20 transferFrom function returns a boolean value indicating success (true) or failure (false). These code segments do not check the return value, assuming transfers always succeed. However, tokens like USDT (on mainnet) or non-compliant implementations may return false instead of reverting on failure. This could result in:

  • Users retaining tokens while the contract logic proceeds as if the transfer succeeded.

  • Incorrect accounting (e.g., recording deposits/withdrawals without actual token movements).

Impact

  • Users may lose tokens if the contract logic proceeds under the false assumption that transfers occurred.

  • Contract state inconsistencies (e.g., minting/redeeming shares without token custody).

  • Exploitable scenarios where malicious actors intentionally force transfer failures.

Tools Used

  • foundry

Recommendations

  1. Explicit Return Value Checks:
    Wrap transferFrom calls in require statements to enforce successful transfers:

    require(
    _shortTokenContract.transferFrom(msg.sender, address(this), _positionTokenAmountToRemove),
    "Short token transfer failed"
    );
  2. Use SafeERC20 Library:
    Replace raw transferFrom calls with OpenZeppelin’s safeTransferFrom from the SafeERC20 library. This handles both boolean returns and non-compliant tokens:

    import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
    // ...
    SafeERC20.safeTransferFrom(_shortTokenContract, msg.sender, address(this), _positionTokenAmountToRemove);
  3. Test with Non-Compliant Tokens:
    Validate behavior with tokens that return false on failure (e.g., USDT on Ethereum mainnet).

Updates

Lead Judging Commences

bube Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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