sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

8olidity - TransferUnderlyingToVaultDirect () error may lead to vault not receive money #39

Closed rcstanciu closed 2 years ago

rcstanciu commented 2 years ago

8olidity

medium

TransferUnderlyingToVaultDirect () error may lead to vault not receive money

Summary

TransferUnderlyingToVaultDirect () error may lead to vault not receive money

Vulnerability Detail

TransferUnderlyingToVaultDirect () will the ETH sent to vault address, but he is calling transferNativeTokenOut (), this function has no check whether the transfer is successful, It is also recommended to use call() instead of ### transfer() to transfer ETH

// contracts-v2/contracts/internal/vaults/VaultConfiguration.sol
function transferUnderlyingToVaultDirect(
    VaultConfig memory vaultConfig,
    address transferFrom,
    uint256 depositAmountExternal
) internal returns (uint256) {
    if (depositAmountExternal == 0) return 0;

    Token memory assetToken = TokenHandler.getAssetToken(vaultConfig.borrowCurrencyId);
    Token memory underlyingToken = assetToken.tokenType == TokenType.NonMintable ? 
        assetToken :
        TokenHandler.getUnderlyingToken(vaultConfig.borrowCurrencyId);

    address vault = vaultConfig.vault;
    // Tokens with transfer fees are not allowed in vaults
    require(!underlyingToken.hasTransferFee);
    if (underlyingToken.tokenType == TokenType.Ether) {
        require(msg.value == depositAmountExternal, "Invalid ETH");
        // Forward all the ETH to the vault
        GenericToken.transferNativeTokenOut(vault, msg.value);

// contracts-v2/contracts/internal/balances/protocols/GenericToken.sol
function transferNativeTokenOut(
  address account,
  uint256 amount
) internal {
  // This does not work with contracts, but is reentrancy safe. If contracts want to withdraw underlying
  // ETH they will have to withdraw the cETH token and then redeem it manually.
  payable(account).transfer(amount);
}

Impact

TransferUnderlyingToVaultDirect () error may lead to vault not receive money

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/contracts-v2/contracts/internal/vaults/VaultConfiguration.sol#L454

Tool used

vscode Manual Review

Recommendation

1.check return 2.use call()

Duplicate of #63