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

Not using safe version of ERC20 transferFrom and approve can lead to unexpected behaviour on Ethereum Mainnet

Summary

In AaveDIVAWrapperCore.sol, several instances of the ERC20 functions transferFrom and approve are used. The standard ERC20 implementation expects these functions to return a boolean value. However, some ERC20 tokens—such as USDT on Ethereum Mainnet—deviate from this standard and do not return a value.

Vulnerability Details

In the _registerCollateralToken function:

https://github.com/Cyfrin/2025-01-diva/blob/1b6543768c341c2334cdff87b6dd627ee2f62c89/contracts/src/AaveDIVAWrapperCore.sol#L115-L116

The approve function is used to set unlimited approval for wToken transfers to the DIVA Protocol and collateral token transfers to Aave V3. This will cause a revert if the target ERC20 token follows a non-standard implementation with a different function signature for approve(). Tokens like USDT will cause this function to revert.

For reference, USDT's approve implementation on Ethereum Mainnet:

https://etherscan.io/token/0xdac17f958d2ee523a2206206994597c13d831ec7#code

Non-standard token like USDT does not return a value, while the default ERC20 behavior expects the approve function to return a boolean:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/840c974028316f3c8172c1b8e5ed67ad95e255ca/contracts/token/ERC20/IERC20.sol#L67

Since USDT does not return a value, the approval call will fail.

Additionally, in the _removeLiquidity and _redeemPositionToken functions:

https://github.com/Cyfrin/2025-01-diva/blob/1b6543768c341c2334cdff87b6dd627ee2f62c89/contracts/src/AaveDIVAWrapperCore.sol#L229-L230

https://github.com/Cyfrin/2025-01-diva/blob/1b6543768c341c2334cdff87b6dd627ee2f62c89/contracts/src/AaveDIVAWrapperCore.sol#L285-L289

The ERC20 function transferFrom is used to transfer tokens from the user to the contract instead of its safer alternative from SafeERC20safeTransferFrom. Using safeTransferFrom ensures proper revert on failure.

The claim here is:

"No need to use safeTransferFrom here as (short and long/position) tokens in the DIVA Protocol are standard ERC20 tokens using OpenZeppelin's ERC20 implementation."

Since the ERC20 interface expects transferFrom to return a boolean, https://github.com/OpenZeppelin/openzeppelin-contracts/blob/840c974028316f3c8172c1b8e5ed67ad95e255ca/contracts/token/ERC20/IERC20.sol#L78

But here is the transferFrom implementation for USDT on the main net and doesn't return a value, https://etherscan.io/token/0xdac17f958d2ee523a2206206994597c13d831ec7#code.

calling transferFrom on tokens that do not return a boolean will result in a revert.

Impact

Both approve and transferFrom calls will revert due to missing return values, breaking the intended functionality of the protocol on Ethereum Mainnet.

Tools Used

  • Manual Review

  • Solodit

Recommendations

As mentioned in this comment:

https://github.com/Cyfrin/2025-01-diva/blob/1b6543768c341c2334cdff87b6dd627ee2f62c89/contracts/src/AaveDIVAWrapperCore.sol#L364-L365

Certain tokens, like USDT, require setting approvals to zero before assigning a new value.

"Tokens like USDT on Ethereum require the approval to be set to zero before setting it to a non-zero value."

To address this:

  • Use OpenZeppelin’s SafeERC20.forceApprove.

  • Also, replace transferFrom with safeTransferFrom.

Updates

Lead Judging Commences

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

Support

FAQs

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