Omniscia Euler Finance Audit

SwapHub Manual Review Findings

SwapHub Manual Review Findings

SHB-01M: Incorrect Execution Mode Assumption

TypeSeverityLocation
Logical FaultSwapHub.sol:L128, L146

Description:

The swapInternal function assumes that the params.mode variable will either be set to 0 or 1, however, this is not the case as it is an arbitrary uint256 variable that can be set to any value.

Impact:

The severity of the finding is set as unknown due to the presence of out-of-scope function invocations that have an unmeasured degree of impact to the codebase.

Example:

contracts/modules/SwapHub.sol
127uint amountOutMin;
128if (params.mode == 0) {
129    amountOutMin = params.amountOut;
130} else {
131    require(params.amountOut > params.exactOutTolerance, "e/swap-hub/exact-out-tolerance");
132    unchecked { amountOutMin = params.amountOut - params.exactOutTolerance; }
133}
134

135require(postBalanceOut >= cache.preBalanceOut + amountOutMin, "e/swap-hub/insufficient-output");
136require(cache.preBalanceIn >= postBalanceIn, "e/swap-hub/positive-input");
137

138uint amountIn;
139uint amountOut;
140unchecked {
141    amountIn = cache.preBalanceIn - postBalanceIn;
142    amountOut = postBalanceOut - cache.preBalanceOut;
143}
144

145// for exact output calculate amounts in post swap. Also when amount sold is not equal to requested (e.g. partial fill)
146if (params.mode == 1 || amountIn != params.amountIn) {
147    amountInScaled = decodeExternalAmount(cache.assetCacheIn, amountIn);
148    amountInInternal = underlyingAmountToBalanceRoundUp(cache.assetCacheIn, amountInScaled);
149}

Recommendation:

We advise the logic of the contract to properly only handle a case of 0 or "Ø" by simply handling the case of params.mode == 0 and params.mode != 0 instead as otherwise amount scaling and amount internal overwrites may be performed incorrectly during a normal swap operation.

Alleviation:

A require check was introduced in the executeSwap function ensuring that the params.mode execution mode is less-than-or-equal-to (<=) the value of 1, properly sanitizing its value and ensuring the logic of the function executes as expected.

SHB-02M: Potential Misbehaviour of Memory Structs

Description:

Although not presently an issue within the contract, any adjustments performed to a memory struct that is passed in as an argument to an internal function will be reflected at the caller's context.

Example:

contracts/modules/SwapHub.sol
111function swapInternal(SwapCache memory cache, address swapHandler, ISwapHandler.SwapParams memory params) private returns (uint) {
112    // Adjust requested amount in
113    (uint amountInScaled, uint amountInInternal) = withdrawAmounts(eTokenLookup[cache.eTokenIn], cache.assetCacheIn, cache.accountIn, params.amountIn);
114    require(cache.assetCacheIn.poolSize >= amountInScaled, "e/swap-hub/insufficient-pool-size");
115    params.amountIn = amountInScaled / cache.assetCacheIn.underlyingDecimalsScaler;
116

117    // Supply handler, for exact output amount transfered serves as an implicit amount in max.
118    Utils.safeTransfer(params.underlyingIn, swapHandler, params.amountIn);
119

120    // Invoke handler
121    ISwapHandler(swapHandler).executeSwap(params);
122

123    // Verify output received, credit any returned input
124    uint postBalanceIn = callBalanceOf(cache.assetCacheIn, address(this));
125    uint postBalanceOut = callBalanceOf(cache.assetCacheOut, address(this));
126

127    uint amountOutMin;
128    if (params.mode == 0) {
129        amountOutMin = params.amountOut;
130    } else {
131        require(params.amountOut > params.exactOutTolerance, "e/swap-hub/exact-out-tolerance");
132        unchecked { amountOutMin = params.amountOut - params.exactOutTolerance; }
133    }
134

135    require(postBalanceOut >= cache.preBalanceOut + amountOutMin, "e/swap-hub/insufficient-output");
136    require(cache.preBalanceIn >= postBalanceIn, "e/swap-hub/positive-input");
137

138    uint amountIn;
139    uint amountOut;
140    unchecked {
141        amountIn = cache.preBalanceIn - postBalanceIn;
142        amountOut = postBalanceOut - cache.preBalanceOut;
143    }
144

145    // for exact output calculate amounts in post swap. Also when amount sold is not equal to requested (e.g. partial fill)
146    if (params.mode == 1 || amountIn != params.amountIn) {
147        amountInScaled = decodeExternalAmount(cache.assetCacheIn, amountIn);
148        amountInInternal = underlyingAmountToBalanceRoundUp(cache.assetCacheIn, amountInScaled);
149    }
150

151    // Process withdraw
152    cache.assetCacheIn.poolSize = decodeExternalAmount(cache.assetCacheIn, postBalanceIn);
153    decreaseBalance(eTokenLookup[cache.eTokenIn], cache.assetCacheIn, cache.eTokenIn, cache.accountIn, amountInInternal);
154    logAssetStatus(cache.assetCacheIn);
155

156    return amountOut;
157}

Recommendation:

We advise this to be taken into account when using this paradigm in future versions of the codebase as currently the adjustment performed to the cache.assetCacheIn.poolSize within swapInternal is reflected in both swap and swapAndRepay albeit with no impact.

Alleviation (59953f2625):

The Euler Finance team stated that the current behaviour is intentional as:

The mutations to AssetCache is an intentional pattern used across the code base as these values are meant to reflect the global state of an asset.

However, the mutation performed on cache.assetCacheIn.poolSize as referenced here is actually not reflected on the global state as the memory struct is never stored to storage explicitly. We advise this exhibit to be re-evaluated in light of this comment unless the storage entry is properly updated within the decreaseBalance or logAssetStatus functions that were not in scope.

Alleviation (59953f2625):

The Euler Finance team re-evaluated the exhibit and stated that all relevant functions that make use of the data point do so appropriately and the poolSize property does not need to be stored in storage directly. Based on the fact that this type of pass-by-memory paradigm is presented throughout the Euler Finance codebase and that the linked instance does not represent an active issue, we consider this exhibit nullified.

Prev

SwapHandlerUniswapV3.sol (SHU-M)

Next

SwapHandlerCombinedBase.sol (SHC-C)

On this page