Omniscia Euler Finance Audit
BalanceUtils Manual Review Findings
BalanceUtils Manual Review Findings
BUL-01M: Potentially Insecure Balance Tracker Invocation
Type | Severity | Location |
---|---|---|
Language Specific | BalanceUtils.sol:L122-L129 |
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:
122function tryBalanceTrackerHook(address account, uint256 newAccountBalance, bool forfeitRecentReward)123 private124 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.