LI.FI contest - shenwilly's results

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/GenericSwapFacet.sol#L30

Vulnerability details

Impact

A faulty input in GenericSwapFacet.swapTokensGeneric() could cause funds to get stuck in the contract.

In addition, tokens left in the LiFi contract can be retrieved by anyone (see issue: ERC20 withdrawals can be frontrun), leading to loss of fund.)

Proof of Concept

1. Alice calls swapTokensGeneric(), inputting WETH as lifiData.receivingAssetId, USDC as swapData.sendingAssetId, and USDT as swapData.receivingAssetId.
2. The contract swaps Alice's USDC to USDT, but tries to send WETH instead of USDT.
3. Alice receives nothing and the USDT is now stuck in the contract.

Recommended Mitigation Steps

Ensure that lifiData.receivingAssetId is equal to receivingAssetId of the final _swapData.

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L17-L26 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/GenericSwapFacet.sol#L22-L43

Vulnerability details

Impact

GenericSwapFacet.swapTokensGeneric() can run arbitrary calls on any whitelisted addresses. A malicious/compromised governance can whitelist token contracts and steal funds from users that have given allowance to LiFi contract.

Proof of Concept

1. Alice approves infinite USDC allowance to LiFi.
2. Governance calls addDex(), whitelisting USDC contract.
3. Governance calls swapTokensGeneric() with USDC as the callTo and transferFrom as the callData, stealing Alice's fund.

Recommended Mitigation Steps

Recommend using timelock when managing dexWhitelist.

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/GenericSwapFacet.sol#L22-L43 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L33-L35

Vulnerability details

Impact

Tokens sent to the contract can be retrieved by anyone, which opens up the possibility for WithdrawFacet.withdraw() to be frontrun.

When swapping ERC20 tokens, LibSwap.swap only tries to transfer tokens if the contract doesn't have enough balance to swap. As long as a market for the token exists, anyone can call GenericSwapFacet.swapTokensGeneric() to swap any token in the contract and retrieve them.

Proof of Concept

1. Alice mistakenly sent 1000 USDC to LiFi contract.
2. Governance tries to rescue the USDC by calling WithdrawFacet.withdraw().
3. An attacker front run the transaction by calling GenericSwapFacet.swapTokensGeneric(), using USDC as sendingAssetId and WETH as receivingAssetId. The attacker receives free WETH.

Recommended Mitigation Steps

Consider transferring the token (sendingAssetId of the first _swapData) to the contract before executing the swaps. Return the value of toAmount in LibSwap.swap() and use it as the fromAmount value for the next swap. This way an attacker cannot utilise funds left in the contract for swapping.