Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/Swapper.sol#L12 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Libraries/LibSwap.sol#L33

Vulnerability details

Impact

LibSwap.swap only transfers non-native tokens from msg.sender when balance of said token is lesser than requested fromAmont, thus if there are any remaining tokens in the contract, attacker can easily utilize those tokens for swapping and in turn claim ownership of those

Proof of Concept

The following exploit is demonstrated using GenericSwapFacet. But the vulnerability can be triggered from almost every faucet that uses _executeSwaps.

Assume that the WETH balance of the contract is 100 WETH
Alice call GenericSwapFacet.swapTokensGeneric and _lifiData.receivingAssetId is set to native asset (can be set to any asset except WETH)

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

    function swapTokensGeneric(LiFiData memory _lifiData, LibSwap.SwapData[] calldata _swapData) public payable {
        uint256 receivingAssetIdBalance = LibAsset.getOwnBalance(_lifiData.receivingAssetId);

        // Swap
        _executeSwaps(_lifiData, _swapData);

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

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

        ...
    }

Alice uses WETH as sendingAsset. And the fromAmount is 100. Since LibAsset.getOwnBalance(fromAssetId) < fromAmount, no WETH will be collected from Alice
The contract swap out 100 WETH to DEX, and swap in some native assets. Then it will send the native assets received DEX to Alice.
Obviously, Alice paid nothing(or only gas) to get the assets.

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

    function swap(bytes32 transactionId, SwapData calldata _swapData) internal {
        uint256 fromAmount = _swapData.fromAmount;
        uint256 toAmount = LibAsset.getOwnBalance(_swapData.receivingAssetId);
        address fromAssetId = _swapData.sendingAssetId;
        if (!LibAsset.isNativeAsset(fromAssetId) && LibAsset.getOwnBalance(fromAssetId) < fromAmount) {
            LibAsset.transferFromERC20(fromAssetId, msg.sender, address(this), fromAmount);
        }

        if (!LibAsset.isNativeAsset(fromAssetId)) {
            LibAsset.approveERC20(IERC20(fromAssetId), _swapData.approveTo, fromAmount);
        }

        // solhint-disable-next-line avoid-low-level-calls
        (bool success, bytes memory res) = _swapData.callTo.call{ value: msg.value }(_swapData.callData);
        if (!success) {
            string memory reason = LibUtil.getRevertMsg(res);
            revert(reason);
        }

        ...
    }

Tools Used

Manual code review.

Recommended Mitigation Steps

Before doing _executeSwaps. the contract should track the preBalance.

And use

prebalance - LibAsset.getOwnBalance(fromAssetId) < fromAmount

instead of

LibAsset.getOwnBalance(fromAssetId) < fromAmount

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