sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

ethan-crypto - Medium: Usage of depreciated .transfer() method in redeemFromNotional can result in revert, especially for flash loan liquidator accounts. #108

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ethan-crypto

medium

Medium: Usage of depreciated .transfer() method in redeemFromNotional can result in revert, especially for flash loan liquidator accounts.

Summary

The redeemFromNotional method on the BaseStrategyVault that the BalancerStrategyBase inherits uses the transfer method to send ether to receiver.

Vulnerability Detail

As with most liquidation accounts in Defi, the liquidator accounts that call the deleverageAccount method will most often be smart contracts that use flashloans or flashswaps to cover the initial capital costs required to offset an account's debt position and ensure a profitable liquidation. If one of these liquidation accounts tries to liquidate an account with an ETH debt position the transaction will certainly fail when:

1.The liquidation smart contract account does not implement a payable fallback function. 2.The liquidation smart contract account implements a payable fallback function which uses more than 2300 gas units. 3.The liquidation smart contract account implements a payable fallback function which needs less than 2300 gas units but is called through a proxy that raises the call’s gas usage above 2300.

Impact

Limits liquidator accounts that need more than 2300 units of gas to process inbound eth transfers.

Code Snippet

https://github.com/notional-finance/contracts-v2/blob/cf05d8e3e4e4feb0b0cef2c3f188c91cdaac38e0/contracts/external/actions/VaultAccountAction.sol#L434

https://github.com/notional-finance/contracts-v2/blob/cf05d8e3e4e4feb0b0cef2c3f188c91cdaac38e0/contracts/internal/vaults/VaultConfiguration.sol#L619

https://github.com/notional-finance/leveraged-vaults/blob/master/contracts/vaults/BaseStrategyVault.sol#L181

Tool used

Manual Review

Recommendation

Use call instead of transfer()

Duplicate of #63