LI.FI contest - GeekyLumberjack's results

Lines of code

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

Vulnerability details

Failed transfer with low level call won't revert

Impact

A missing require may cause user to lose funds if a corner case issue filed separately named Failed transfer with low level call won't revert. This may result in user funds lost because funds were transferred into this contract in preparation for the swap. The swap fails but doesn't revert. In GenericSwapFacet.sol there is no require to check that postSwapBalance is greater than 0. However, there is in the other facets.

Proof of Concept

1. Alice uses Generic swap with 100 DAI
2. Alice's 100 DAI are sent to the Swapper.sol contract
3. The call on swap _swapData.callTo.call{ value: msg.value }(_swapData.callData); fails but returns success due to nonexisting contract
4. postSwapBalance = 0 Other facets would revert here
5. Alice receives nothing in return

Tools Used

Manual review

Recommended Mitigation Steps

add require(postSwapBalance > 0, "ERR_INVALID_AMOUNT");

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Libraries/LibSwap.sol#L42-L46

Vulnerability details

Impact

swap is used throughout the code via _executeSwaps in Swapper.sol. According to Solidity Docs the call may return true even if it was a failure. This may result in user funds lost because funds were transferred into this contract in preparation for the swap. The swap fails but doesn't revert. There is a way this can happen through GenericSwapFacet.sol due to a missing require that is present in the other facets which is a separate issue but gives this issue more relevance.

Proof of Concept

1. Alice uses Generic swap with 100 DAI
2. Alice's 100 DAI are sent to the Swapper.sol contract
3. The call on swap _swapData.callTo.call{ value: msg.value }(_swapData.callData); fails but returns success due to nonexisting contract
4. postSwapBalance = 0
5. Alice receives nothing in return

Tools Used

Manual review

Recommended Mitigation Steps

Check for contract existence

A similar issue was awarded a medium here