Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibAsset.sol#L59-L70 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L33-L35 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/GenericSwapFacet.sol#L28-L30

Vulnerability details

Using LibAsset.sol approval exploit which LibSwap.sol approve all whitelist address access to transfer all token hold by LiFi contract. reference 1 reference 2
most facet function does not check input LibSwap.SwapData[] before _executeSwaps(_lifiData, _swapData); pass on to LibSwap.sol. This allows anyone executes anything with whitelist DEX using Li.Fi as caller

Impact

Any malicious user can call LibSwap.sol execute to swap all token balance hold by Li.Fi contract to another token. Which can be used to transfer all token hold by Li.Fi contract to another address.

Proof of Concept

This only show the most simple, direct way to withdraw ERC20 token from contract. Consider Li.Fi already implement Whitelist function and token whitelist.

Attacker call swapTokensGeneric() function in GenericSwapFacet.sol
input _lifiData.receivingAssetId is USDC. This cache uint256 receivingAssetIdBalance equal to current USDC token balance.
LibSwap.SwapData[] _swapData is custom compose to swap all token hold by Li.Fi contract to USDC through Uniswap.
callTo and approveTo is Uniswap router address (whitelisted in dex.ts)
sendingAssetId, receivingAssetId and fromAmount in SwapData can be any value
bytes callData in LibSwap.SwapData is simply swapExactTokensForTokens with Uniswap
Uniswap router have max approval token transfer, so it can call transferFrom to get all Li.Fi token directly.
Li.Fi transfer all token to Uniswap router, then Uniswap Swap to USDC transfer back to Li.Fi contract.
Do this for all token hold Li.Fi in array swap
After postSwapBalance check. Li.Fi contract transfer all new swapped USDC to attacker. (attacker only have to spend 0.1 of any random ERC20 token)

Do this again with USDC and USDT to withdraw all USDC leftover.

Note: there are many more ways to exploit this using the same generic LibSwap.swap function. As long as Li.Fi contract hold ERC-20 token, there is no safe or fair way to prevent attacker make unfair trade with token.

Tools Used

Manual

Recommended Mitigation Steps

The best fix from my experience is to remove LibSwap.sol and restart from scratch. Try different approach instead of generic arbitrary call to mitigate this. Spending time on securing LibSwap.sol is simply not worth it.

From LibSwap documentation here, A library used to perform multiple swaps given an array of DEX addresses and calldata. This makes it very flexible but also dangerous in that it allows arbitrary calls to any contract

This is a wrong approach with diamond facet. Diamond facets are already flexible, no need for arbitrary call. Diamond allow adhoc implementation for each chain, different contracts which give flexibility to implement new features or new DEX. Sharing all swap logic into one single function and reuse it everywhere is not a good idea.

Current implementation, Swapper.sol raw call in loop, is no different from a multicall function to different DEX to swap and then cross bridge. Might as well use a multicall into facets function instead of facet use same lib function. It is much simpler structure wise.

For flashloan swap, as long as Li.Fi not check correct amount of token being swapped, attacker can use other people token like token hold by LiFi contract.

code-423n4 / 2022-03-lifinance-findings