LI.FI contest - TomFrenchBlockchain's results

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/Swapper.sol#L14-L21 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L42

Vulnerability details

Impact

_executeSwaps will revert if multiple swaps are performed and msg.value > 0, preventing the use of the native asset.

Proof of Concept

A core feature of DEX aggregators is that they allow users to split a single trade across multiple DEXs, this is the intention behind the _executeSwaps function.

However when executing a swap in the LibSwap library it attaches the full value of msg.value onto all external calls to DEXs. This means that unless each DEX returns msg.value of the native asset back to Lifi.sol then the next call will revert due to insufficient ETH to attach to the call. The protocol will then not function for a core capability.

Note this also allows users to drain any ETH which is left sitting on the Lifi contract by setting msg.value equal to Lifi's balance and performing 2 trades which use ETH as the input and sending the output to their address (leaking value).

Recommended Mitigation Steps

Rather than attaching msg.value to the external call unconditionally, only add fromAmount if LibAsset.isNativeAsset(fromAssetId)

Add tests for native asset handling to the test suite.