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

ERC20.approve Used Instead of Safe Approvals, Causing `registerCollateralToken()` Failures with Some ERC20s

Summary

The function _registerCollateralToken handles collateral tokens registration, It's using "ERC20.approve" to approves the Aave V3 contract to transfer the collateralToken registered in _handleTokenOperations.

However, some ERC20s on some chains don't return a value.
The most popular example is USDT and USDC on the main net, and as the docs mention it should be compatible on any EVM chain and will support USDT:

the current implementation of the approve function does not account for tokens like USDT, contradicting the protocol's intentions.
Therefore _registerCollateralToken will never work on the EVM Chain or other related chain and tokens.

Vulnerability Details

In the _registerCollateralToken:115 , approve function is used instead of safeApprove. USDT on the main net doesn't return a value, https://etherscan.io/token/0xdac17f958d2ee523a2206206994597c13d831ec7#code. This includes USDC which should work in the protocol.
This behavior causes the approve function to revert when interacting with these tokens

function _registerCollateralToken(address _collateralToken) internal returns (address) {
// Verify that the collateral token is not yet registered.
if (_collateralTokenToWToken[_collateralToken] != address(0)) {
revert CollateralTokenAlreadyRegistered();
}
// Retrieve the aToken address associated with the provided collateral token from Aave V3. Reverts if
// the collateral token is not supported by Aave V3.
// Note: aTokens have the same number of decimals as the collateral token: https://discord.com/channels/602826299974877205/636902500041228309/1249607036417867810
address _aToken = _getAToken(_collateralToken);
if (_aToken == address(0)) {
revert UnsupportedCollateralToken();
}
IERC20Metadata _collateralTokenContract = IERC20Metadata(_collateralToken);
// Deploy a token that represents a wrapped version of the collateral token to be used as proxy collateral in DIVA Protocol.
// The symbol and name of the wToken are derived from the original collateral token, prefixed with 'w' (e.g., wUSDT or wUSDC).
// This naming convention helps in identifying the token as a wrapped version of the original collateral token.
// The wToken decimals are aligned with those of the collateral token and the aToken.
// This contract is set as the owner and has exclusive rights to mint and burn the wToken.
WToken _wTokenContract = new WToken(
string(abi.encodePacked("w", _collateralTokenContract.symbol())),
_collateralTokenContract.decimals(),
address(this) // wToken owner
);
address _wToken = address(_wTokenContract);
// Map collateral token to its corresponding wToken.
_collateralTokenToWToken[_collateralToken] = _wToken;
// Map wToken to its corresponding collateral token to facilitate reverse lookups.
_wTokenToCollateralToken[_wToken] = _collateralToken;
// Set unlimited approval for the wToken transfer to DIVA Protocol and the collateral token transfer to Aave V3. This setup reduces
// the need for repeated approval transactions, thereby saving on gas costs.
// The unlimited approvals are deemed safe as the `AaveDIVAWrapper` is a pass-through entity that does not hold excess wTokens or collateral tokens.
// Should a vulnerability be discovered in DIVA Protocol or Aave, users can simply stop interacting with the `AaveDIVAWrapper` contract.
//
// Note that granting an infinite allowance for wToken does not reduce the allowance on `transferFrom` as it uses a newer OpenZeppelin ERC20 implementation.
// However, this behavior may differ for collateral tokens like USDC, DAI, or WETH used in Aave. These tokens decrement the allowance with each use of
// `transferFrom`, even if an unlimited allowance is set. Consequently, though very unlikely, AaveDIVAWrapper could eventually exhaust its allowance.
// The `approveCollateralTokenForAave` function has been implemented to manually reset the allowance to unlimited.
_wTokenContract.approve(_diva, type(uint256).max);
// @audit appproving some contract might be weird
_collateralTokenContract.approve(_aaveV3Pool, type(uint256).max);
emit CollateralTokenRegistered(_collateralToken, _wToken);
return _wToken;
}

The specific part of the function is highlighted here;

_collateralTokenContract.approve(_aaveV3Pool, type(uint256).max);
emit CollateralTokenRegistered(_collateralToken, _wToken);

Internal pre-conditions

  1. The protocol uses approve calls in _registerCollateralToken .

  2. There is no consideration or error handling for approve calls when interacting with USDT, USDC.

External pre-conditions

  1. The protocol operates on EVM-compatible chains where these tokens are prevalent.

Impact

Attack Path

  1. Owner wants to use USDT or USDC as the colateralToken.

  2. _registerCollateralToken that include approve call revert, causing the protocol to fail in providing the intended functionality.

  3. The protocol becomes unusable with ERC20 tokens like USDT AND USDC, a widely used stablecoin.

  4. Breaks the intended functionality of the protocol.

  5. This breaks a criticial function in the protocol and causes the `_registerCollateralToken()' to fail.

POC

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
import "forge-std/Test.sol";
import "../src/AaveDIVAWrapper.sol";
import "../src/mocks/MockERC20.sol";
import "../src/interfaces/IAave.sol";
import "../src/interfaces/IDIVA.sol";
contract AaveDiverWrapperTest is Test {
AaveDIVAWrapper aaveDIVAWrapper;
IAave aaveEth = IAave(address(0x87870Bca3F3fD6335C3F4ce8392D69350B4fA4E2));
IDIVA DIVAEth = IDIVA(address(0x2C9c47E7d254e493f02acfB410864b9a86c28e1D));
MockERC20 collateralTokenUSDTEth =
MockERC20(address(0xdAC17F958D2ee523a2206206994597C13D831ec7));
address owner = makeAddr("owner");
function setUp() public {
aaveDIVAWrapper = new AaveDIVAWrapper(
address(DIVAEth),
address(aaveEth),
owner
);
}
function testFailApproveFailWIthONCHAINUSDT() public {
vm.prank(owner);
aaveDIVAWrapper.registerCollateralToken(address(collateralTokenUSDTEth));
}
}

run the above code in an empty test file with forge t --fork-url https://ethereum.rpc.subquery.network/public -vvvvv

│ ├─ [26953] 0xdAC17F958D2ee523a2206206994597C13D831ec7::approve(0x87870Bca3F3fD6335C3F4ce8392D69350B4fA4E2, 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
│ │ ├─ emit Approval(owner: AaveDIVAWrapper: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], spender: 0x87870Bca3F3fD6335C3F4ce8392D69350B4fA4E2, value: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
│ │ ├─ storage changes:
│ │ │ @ 0x14eeb4b4c3069a8bb270c7ef3e4915e3192424d4f11f50bed308a86d7a3635cc: 00xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
│ │ └─ ← [Stop]
│ ├─ storage changes:
│ │ @ 4: 1 → 2
│ │ @ 0xf4f699a1ef35e64890828cc562d137cceeab28af8455567ef14b6ec385c8e441: 00x000000000000000000000000104fbc016f4bb334d775a19e8a6510109ac63e00
│ │ @ 0xbc93b95fb13b8fd819fbf60078dbe0264f9841ed2d94ee86afa3a1c5655d236b: 00x000000000000000000000000dac17f958d2ee523a2206206994597c13d831ec7
│ └─ ← [Revert] EvmError: Revert
└─ ← [Revert] EvmError: Revert

it can be seen above, after the approval for DIVA is done successfully, the Approval for AAve fail and cause the whole function to revert,

Tools Used

Manual Review

Recommendations

Use safeApprove instead of approve

- _collateralTokenContract.approve(_aaveV3Pool, type(uint256).max);
+ _collateralTokenContract.safeApprove(_aaveV3Pool, type(uint256).max);
emit CollateralTokenRegistered(_collateralToken, _wToken);
Updates

Lead Judging Commences

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

Support

FAQs

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

Give us feedback!