hats-finance / VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f

LP token lending protocol
MIT License
2 stars 1 forks source link

Lack of the function to rescue tokens locked in the AToken contract #51

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @0xmuxyz Submission hash (on-chain): 0x40615071430dafea884460e85f117ee177695bf611ffb78caf17e82f44a0efc0 Severity: medium severity

Description:

Description

Within the original AToken contract in the Aave protocol , the rescueTokens() function would be defined in order to rescue and transfer tokens locked in this contract like this: https://github.com/aave/aave-v3-core/blob/0f5ed35a842f5bb06c8ce7febf8c3f0ee4370859/contracts/protocol/tokenization/AToken.sol#L252-L255

  function rescueTokens(address token, address to, uint256 amount) external override onlyPoolAdmin {
    require(token != _underlyingAsset, Errors.UNDERLYING_CANNOT_BE_RESCUED);
    IERC20(token).safeTransfer(to, amount);
  }

According to the NatSpec of the rescueTokens() in the IAToken contract in the Aave protocol, this rescueTokens() function would be implemented in order for a PoolAdmin user to rescue and transfer tokens locked in this contract like this. https://github.com/aave/aave-v3-core/blob/0f5ed35a842f5bb06c8ce7febf8c3f0ee4370859/contracts/interfaces/IAToken.sol#L132-L137

  /**
   * @notice Rescue and transfer tokens locked in this contract
   * @param token The address of the token
   * @param to The address of the recipient
   * @param amount The amount of token to transfer
   */
  function rescueTokens(address token, address to, uint256 amount) external;

However, within the AToken (vToken) contract in the VMEX protocol, the rescueTokens() function would not be implemented. https://github.com/hats-finance/VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f/blob/master/packages/contracts/contracts/protocol/tokenization/AToken.sol

Impact

This lead to that if some tokens are locked (left) in the AToken contract, a PoolAdmin can not rescue these tokens locked in this contract. As a result, these tokens would be locked inside the AToken (vToken) contract forever.

Recommendations

Within the AToken (vToken) contract, consider implementing the rescueTokens() so that a PoolAdmin user can rescue and transfer tokens locked in this contract.

ksyao2002 commented 1 year ago

Potential duplicate of https://github.com/hats-finance/VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f/issues/8

Also, we are a fork of Aave v2, which does not include the rescueTokens() function. Please see the reasoning behind not including that:

"This is not a security concern, but rather a matter of preference. Enabling trusted actors to withdraw funds out of the protocol comes with its own risks, which is why Aave v2 did not include such an ability to begin with."