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.
In the _registerCollateralToken function:
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:
Since USDT does not return a value, the approval call will fail.
Additionally, in the _removeLiquidity and _redeemPositionToken functions:
The ERC20 function transferFrom is used to transfer tokens from the user to the contract instead of its safer alternative from SafeERC20—safeTransferFrom. 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.
Both approve and transferFrom calls will revert due to missing return values, breaking the intended functionality of the protocol on Ethereum Mainnet.
Manual Review
Solodit
As mentioned in this comment:
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.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.