All swapping functions lack checks for returned tokens

[code423n4]

Lines of code

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

Vulnerability details

Impact

Every function that stems from the GenericSwapFacet lacks checks to ensure that some tokens have been returned via the swaps. In LibSwap.sol in the swap() function, the swap call is sent to the target DEX. A return of success is required, otherwise the operation will revert.

Each "inner" swap via LibSwap.sol lacks output checks and also the "outer" swapTokensGeneric() via GenericSwapFacet.sol lacks a final check as well.

There is a possibility that the calldata is accidently populated with a function in the target router that is not actually performing any swapping functionality, getAmountsOut() for example. The function will return true, but no new returned tokens will be present in the contract. Meanwhile, the contract has already received the user's fromTokens directly.

Tools Used

Manual Review

Recommended Mitigation Steps

This would be a potential use case of using function signature whitelists as opposed to contract address whitelists, as noted as a possibility by the LiFi team.

Otherwise, the following require statement in swapTokensGeneric() would ensure that at least a single token was received:

require(LibAsset.getOwnBalance(_swapData.receivingAssetId) - toAmount) > 0, "No tokens received")

[H3xept]

Fixed in lifinance/lifi-contracts@3a42484dda8bafcfd122c8aa3b61d3766d545bf9

[gzeoneth]

Sponsor confirmed with fix.