Omniscia Euler Finance Audit

SwapHandler1Inch Manual Review Findings

SwapHandler1Inch Manual Review Findings

SHI-01M: Arbitrary Payload Invocations

Description:

The swapPrimary function relays an arbitrary payload to the oneInchAggregator via the call instruction which should be considered unsafe depending on the functions exposed by the oneInchAggregator contract (i.e. the amountOut decoded may be ultimately incorrect).

Example:

contracts/swapHandlers/SwapHandler1Inch.sol
15function swapPrimary(SwapParams memory params) override internal returns (uint amountOut) {
16 setMaxAllowance(params.underlyingIn, params.amountIn, oneInchAggregator);
17
18 (bool success, bytes memory result) = oneInchAggregator.call(params.payload);
19 if (!success) revertBytes(result);
20
21 // return amount out reported by 1Inch. It might not be exact for fee-on-transfer or rebasing tokens.
22 amountOut = abi.decode(result, (uint));
23}

Recommendation:

We advise the swapPrimary function to at least validate the params.payload signature and only allow a sub-set of function signatures of oneInchAggregator to be invoked. Alternatively, we advise the swapPrimary to be refactored to not utilize a call instruction at all and to instead use explicit functions exposed by the oneInchAggregator contract by decoding the input arguments contained in params.payload.

Alleviation:

The Euler Finance team evaluated this exhibit and has stated that while arbitrary code execution can be dangerous, in this particular case of 1inch it is unavoidable and thus acceptable. This is due to the fact that the payload that is constructed for a 1inch trade through their API utilizes an undisclosed methodology and as such cannot be validated. We would like to note that simple function signature validation can be performed, however, we understand that the Euler Finance team wish to minimize gas costs and that the impact of arbitrary code execution on behalf of this particular module does not pose an active threat. As a result, we will address this exhibit as nullified based on the fact that 1inch does not disclose their payload construction methodology.