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

Unsafe ERC20 Operations Used Instead of SafeERC20

Summary

The contract performs direct ERC20 token operations without using SafeERC20 wrapper functions. While some tokens are known to use standard implementations, others (particularly collateral tokens) could be non-standard ERC20 tokens that don't properly return values or have unusual behavior.

Vulnerability Details

The following instances were found:

  1. In AaveDIVAWrapperCore.sol:

// Direct approve calls without using safeApprove
_wTokenContract.approve(_diva, type(uint256).max);
_collateralTokenContract.approve(_aaveV3Pool, type(uint256).max);
// Direct transferFrom calls without using safeTransferFrom
_shortTokenContract.transferFrom(msg.sender, address(this), _positionTokenAmountToRemove);
_longTokenContract.transferFrom(msg.sender, address(this), _positionTokenAmountToRemove);
_positionTokenCon
tract.transferFrom(msg.sender, address(this), _positionTokenAmountToRemove);

While the contract already imports and uses SafeERC20 in some places:

using SafeERC20 for IERC20Metadata;

It doesn't consistently apply it across all ERC20 operations.

Impact

  • Low severity since most modern tokens behave correctly, but some tokens (like USDT) are known to have non-standard behaviors

  • Potential for failed operations not being detected if tokens don't properly return success values

  • Inconsistent handling of approvals within the same contract could lead to integration issues

  • Particularly risky with collateral tokens which could be any ERC20 token

Tools Used

Recommendations

  1. Use SafeERC20 consistently throughout the contract for all ERC20 operations:

// For approvals:
_wTokenContract.safeApprove(_diva, type(uint256).max);
// Or better, use safeIncreaseAllowance which is safer for tokens like USDT
_collateralTokenContract.safeIncreaseAllowance(_aaveV3Pool, type(uint256).max);
// For transfers:
_shortTokenContract.safeTransferFrom(msg.sender, address(this), _positionTokenAmountToRemove);
_longTokenContract.safeTransferFrom(msg.sender, address(this), _positionTokenAmountToRemove);
_positionTokenContract.safeTransferFrom(msg.sender, address(this), _positionTokenAmountToRemove);
  1. For consistency with the existing _approveCollateralTokenForAave function, use the following pattern for approvals:

uint256 currentAllowance = _collateralTokenContract.allowance(address(this), _aaveV3Pool);
_collateralTokenContract.safeIncreaseAllowance(_aaveV3Pool, type(uint256).max - currentAllowance);
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.