code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

In the swapTokensGeneric() function, an arbitrary number of swaps can be performed from and to various tokens. However, the final balance that is sent to the user relies on _lifiData.receivingAssetId which has no use in the swapping functionality. LifiData is claimed to be used purely for analytical reasons per the comments to this function. If this value is input incorrectly, the swapped tokens will simply sit in the contract and be lost to the user.

Proof of Concept

Imagine a call to swapTokensGeneric() with the following parameters (excluding unnecessary parameters for this example):

LifiData.receivingAssetId = '0xUSDC_ADDRESS'

Single SwapData array:

LibSwap.SwapData.sendingAssetId = '0xWETH_ADDRESS'
LibSwap.SwapData.receivingAssetId = '0xDAI_ADDRESS'

Since the receivingAssetId from SwapData does not match the receivingAssetId from LifiData, the final funds will not be sent to the user after the swap is complete, based on the following lines of code:

uint256 receivingAssetIdBalance = LibAsset.getOwnBalance(_lifiData.receivingAssetId);

 _executeSwaps(_lifiData, _swapData);

 uint256 postSwapBalance = LibAsset.getOwnBalance(_lifiData.receivingAssetId) - receivingAssetIdBalance;

 LibAsset.transferAsset(_lifiData.receivingAssetId, payable(msg.sender), postSwapBalance);

Lines 1, 3, and 4 reference LifiData.receivingAssetId and handle the transfer of funds following the swaps. Line 2 performs the swap, referencing SwapData.receivingAssetId as can be seen in the executeSwaps() function definition:

function _executeSwaps(LiFiData memory _lifiData, LibSwap.SwapData[] calldata _swapData) internal {
        // Swap
        for (uint8 i; i < _swapData.length; i++) {
            require(
                ls.dexWhitelist[_swapData[i].approveTo] == true && ls.dexWhitelist[_swapData[i].callTo] == true,
                "Contract call not allowed!"
            );

            LibSwap.swap(_lifiData.transactionId, _swapData[i]);
        }
    }

Tools Used

Manual Review.

Recommended Mitigation Steps

I recommend adding a check that _lifiData.receivingAssetId equals the receivingAssetId of the last index of the SwapData array, or simply use the receivingAssetId of the last index of the SwapData array for sending the final tokens to the user.

H3xept commented 2 years ago

Fixed in lifinance/lifi-contracts@52aa2b8ea3bc51de3e60784c00201e29103fe250

gzeoneth commented 2 years ago

Sponsor confirmed with fix.

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