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

6 stars 4 forks source link

Double spend msg.value in Swapper.sol #124

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/Swapper.sol#L14 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L42 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/AnyswapFacet.sol#L85 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/CBridgeFacet.sol#L98 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/GenericSwapFacet.sol#L23 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/NXTPFacet.sol#L91

Vulnerability details

Swapper.sol function _executeSwaps() call in a loop that use the same msg.value sent by user. This allows anyone to call _executeSwaps() which can control Li.Fi contract ETH balance on raw call through function LibSwap.swap().

Note: Lines of code above include all facets swap function check wrong input token before swap. Which anyone can freely call whatever whitelist function on whitelist address without spend their token.

Impact

Attacker can withdraw main token (ETH) from LiFi contract. Dex Whitelist and Function whitelist cannot prevent this

Other User funds are not affected.

Proof of Concept

Attacker can exploit a function that does not check main token balance after _executeSwaps like in AnySwapFacet.sol. These lines of code here only check input balance of AnyswapData memory _anyswapData after execute and not balance of token inside LibSwap.SwapData[] calldata _swapData. (same go for GenericSwapFacet.sol with swapTokensGeneric() and every other facets)

Attacker can craft special msg to DODO RouteProxy (whitelist in dex.ts) and swap Li.Fi ETH to other token and transfer it to another address.

The simpler way is to just swap 1 ETH to USDC with swapTokensGeneric() will just send back 2 ETH worth of USDC with same exploit above.

Note: This concept use other attack approach from different facets instead of attack through Uniswap like other report. Due to some other bugs exist in LibSwap.sol and Swapper.sol that I already found. It is possible swap ETH directly for some fake token.

Tools Used

Manual

Recommended Mitigation Steps

Migrate this might require change all logic in facets.

I would highly recommend create adhoc data function and execution for each facet instead of 1 Swapper.sol for all. This help reduces complexity and help reduce development time in long term. Also, 1 broken library does not require refactoring all facets code. Each facet should be modular and independent of each other.

Each facet should have its own unique SwapData or inputData.

LibSwap.SwapData[] should be a subset data of this SwapData and not a direct function input.

If possible, create new SwapData inside function and not input send by user. Input should be used to create data to pass on LibSwap.sol.

H3xept commented 2 years ago

Duplicate of #86