Omniscia Euler Finance Audit

BalanceUtils Manual Review Findings

BalanceUtils Manual Review Findings

BUL-01M: Potentially Insecure Balance Tracker Invocation

Description:

The BalanceUtils::tryBalanceTrackerHook function is meant to interact with the balanceTracker in a non-prohibitive manner, ensuring that whatever failure may be encountered in the balanceTracker cannot affect the sensitive operations of the BalanceUtils contract.

We do not consider the present safety precautions as adequate due to the following two possibilities:

  • Return-Based Gas Bombs
  • Unbounded Gas Consumption

Despite what the syntax of the BalanceUtils::tryBalanceTrackerHook function infers, any returned payload by the IBalanceTracker will be decoded and in case it is maliciously large will result in an out-of-gas error regardless of the gas stipend supplied to the call.

On the other hand, any unbounded gas operation in the IBalanceTracker (i.e. due to invalid EVM operations, an unbounded loop, propagation of the aforementioned error via the balanceTracker, etc.) will result in the BalanceUtils context and to not be able to complete even if the 63/64 rule is observed.

Depending on the trust level of the configured balanceTracker, the severity of this vulnerability ranges from high-to-moderate as it can either be maliciously imposed or may result inadvertently.

Impact:

The present BalanceUtils::tryBalanceTrackerHook integration with the IBalanceTracker may result in an out-of-gas error, preventing sensitive balance adjustments from being executable.

Example:

src/EVault/shared/BalanceUtils.sol
122function tryBalanceTrackerHook(address account, uint256 newAccountBalance, bool forfeitRecentReward)
123 private
124 returns (bool success)
125{
126 (success,) = address(balanceTracker).call(
127 abi.encodeCall(IBalanceTracker.balanceTrackerHook, (account, newAccountBalance, forfeitRecentReward))
128 );
129}

Recommendation:

We advise the BalanceUtils::tryBalanceTrackerHook function to instead utilize a low-level assembly block to avoid decoding the returned payload unconditionally.

As an added security measure, the code may impose a fixed gas stipend that any IBalanceTracker compliant contract will need to abide by if it wishes to be integrated properly by the Euler Finance system.

Alleviation (fb2dd77a6f):

The Euler Finance team has clarified that the Balance Tracker is a trusted contract and as such is expected to behave properly. We consider this statement to contradict the code's present implementation which will perform a low-level call opportunistically instead of ensuring that the call succeeds under all circumstances.

The code should either adopt a complete-trust model whereby the integration is performed directly without a low-level call and ignorance of the success flag, or a zero-trust model whereby the integration is performed with maximum security sanitizations enabled.

We originally assumed that a no-trust model is meant to be adhered to, and in light of the new information supplied by the Euler Finance team advise that the integration is performed directly by wrapping the balanceTracker in the IBalanceTracker interface and invoking the relevant function.

Alleviation (0f2192ac81):

The Euler Finance team evaluated our follow-up recommendation and proceeded with removing the BalanceUtils::tryBalanceTrackerHook function from the contract, instead performing the IBalanceTracker integration directly as advised.

As such, we consider this exhibit properly nullified as the originally described vulnerability was incorrect based on the complete-trust model that the code presently adheres to.