Infinite token Approval amount bug

[code423n4]

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibAsset.sol#L59-L70 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L38

Vulnerability details

LibAsset.sol approve max for spender instead of replace or add approval amount. This is unintended behavior as swap suppose to use only amount user send to contract and not all token currently hold.

The docs inside LibAsset.sol for approveERC20 is confusing. It says Amount to approve for spending but it actually gives max allowance for spender. This might be error on Connext part since their implementation is different.

Technically, Li.Fi gives all whitelisted address inside config Dex.ts access to transfer all token balance currently hold by Li.Fi.

Note: suppose to be low severity, but there are multiple exploits using this to withdraw all tokens hold by LiFi contract. Also, if attacker have special angle attack in the future, attacker can force LibSwap.sol call router like Uniswap to call transferFrom directly with full approval by Li.Fi to withdraw all tokens.

Impact

Anyone can tell Li.Fi what to do with all token currently hold in contract without owner access.

Ex: attacker can force Li.Fi trade its own token for another token. Because LiFi give Uniswap router approval to spend any token.

Proof of Concept

Several angles of attack can happen because attacker can freely craft swapData to LibSwap.sol swap function without restriction

• Some whitelist DEX like DODO allow transfer token directly to another address after swap.
• Without token whitelist, it is simple to swap valuable token with fake token liquidity and rugpull that liquidity pool.
• Even with limited function whitelist and token whitelist, attacker can still force bad trade using flashloan price manipulation. This allows the contract to spend more token to swap than it should. This render safeTransferERC20 from sender before swap useless, because attacker can swap using fund hold by LiFi contract too.

Tools Used

Manual

Recommended Mitigation Steps

• replace approveERC20 here with increaseERC20Allowance since most ERC20 token support this.
• Due to how liquidity and flashloan work, it is unavoidable of having attacker make a manipulated trade with DEX like Uniswap. The best way is to make sure Li.Fi do not spend unexpected token balance during swap.

[H3xept]

Duplicate of #130

[gzeoneth]

Duplicate of #160