`AnyswapFacet` can be exploited to approve arbitrary tokens.

[code423n4]

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L35-L53

Vulnerability details

Impact

In AnyswapFacet.sol we parse arbitrary data in _anyswapData allowing an attacker to drain funds (ERC20 or native tokens) from the LiFi contract.

Functions effected:

• AnyswapFacet.startBridgeTokensViaAnyswap()
• AnyswapFacet.swapAndStartBridgeTokensViaAnyswap()

Proof of Concept

This attack works in AnyswapFacet.startBridgeTokensViaAnyswap() by having a malicious _anyswapData.token which may change the value return in IAnyswapToken(_anyswapData.token).underlying();.

First we have the first call to IAnyswapToken(_anyswapData.token).underlying(); return a malicious ERC20 contract in the attackers control. This allows for transferring these malicious ERC20 tokens to pass the required balance checks.

            uint256 _fromTokenBalance = LibAsset.getOwnBalance(underlyingToken);
            LibAsset.transferFromERC20(underlyingToken, msg.sender, address(this), _anyswapData.amount);

            require(
                LibAsset.getOwnBalance(underlyingToken) - _fromTokenBalance == _anyswapData.amount,
                "ERR_INVALID_AMOUNT"
            );

The function will then call _startBridge() which again does address underlyingToken = IAnyswapToken(_anyswapData.token).underlying(); we have the malicious _anyswapData.token return a different address, one which the LiFi contract has balance for (a native token or ERC20).

We will therefore execute the following which will either approve or transfer funds to _anyswapData.router for a different underlyingtoken to the one which supplied the funds to LiFi.

        address underlyingToken = IAnyswapToken(_anyswapData.token).underlying();

        if (underlyingToken == IAnyswapRouter(_anyswapData.router).wNATIVE()) {
            IAnyswapRouter(_anyswapData.router).anySwapOutNative{ value: _anyswapData.amount }(
                _anyswapData.token,
                _anyswapData.recipient,
                _anyswapData.toChainId
            );
            return;
        }

        if (_anyswapData.token != address(0)) {
            // Has underlying token?
            if (underlyingToken != address(0)) {
                // Give Anyswap approval to bridge tokens
                LibAsset.approveERC20(IERC20(underlyingToken), _anyswapData.router, _anyswapData.amount);

                IAnyswapRouter(_anyswapData.router).anySwapOutUnderlying(
                    _anyswapData.token,
                    _anyswapData.recipient,
                    _anyswapData.amount,
                    _anyswapData.toChainId
                );
            } else {
                // Give Anyswap approval to bridge tokens
                LibAsset.approveERC20(IERC20(_anyswapData.token), _anyswapData.router, _anyswapData.amount);

                IAnyswapRouter(_anyswapData.router).anySwapOut(
                    _anyswapData.token,
                    _anyswapData.recipient,
                    _anyswapData.amount,
                    _anyswapData.toChainId
                );
            }
        }
    }

Since _anyswapData.router is an address in the attackers control they either are transferred native tokens or they have an allowance of ERC20 tokens that they can spend arbitrarily.

The attack is almost identical in swapAndStartBridgeTokensViaAnyswap()

Recommended Mitigation Steps

Consider whitelisting both Anyswap tokens and Anyswap routers (using two distinct whitelists) restricting the attackers ability to use malicious contracts for this attack.

Consider also only calling IAnyswapToken(_anyswapData.token).underlying() once and passing this value to _startBridge().

[H3xept]

We fixed the issue by not allowing underlying() to be called multiple times in one transaction (lifinance/lifi-contracts@a8d6336c2ded97bdbca65b64157596b33f18f70d)

We disagree with the severity as no funds should ever be left in the contract by design.

[gzeoneth]

Keeping this and all duplicates as Med Risk. There can be fund leftover in the contract under normal operation, for example this tx. In fact, ~$300 worth of token is left in the LI.Fi smart contract on ETH mainnet 0x5a9fd7c39a6c488e715437d7b1f3c823d5596ed1 as of block 14597316. I don't think this is High Risk because the max amount lost is no more than allowed slippage, which can be loss to MEV too.