Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
hats-bug-reporter bot opened this issue Jun 29, 2023 · 1 comment
Open
Labels
bug Something isn't working duplicate This issue or pull request already exists

Comments

@hats-bug-reporter
Copy link

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.

@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Jun 29, 2023
@ksyao2002
Copy link

Potential duplicate of #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."

@ksyao2002 ksyao2002 added the duplicate This issue or pull request already exists label Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

1 participant