search

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

6 stars 4 forks source link

`swap()` May Call `callTo` With any Arbitrary Data #88

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Libraries/LibSwap.sol#L42 https://github.com/Uniswap/v2-core/blob/master/contracts/UniswapV2ERC20.sol#L63

Vulnerability details

Impact

The function LibSwap.swap() allows the user to submit arbitrary data when performing a swap. The impact of this is that a user may call functions that are not intended to be called by the user. One example is the approve() function in UniswapV2.

Since the user may call UniswapV2.approve(address spender, uint value) with arbitrary data they may set themselves as the spender and the amount to (uint256).max allowing the to drain any balances in the contract whenever their may be a balance (such as during another users transaction if they can gain control).

The attacker could alternatively just call UniswapV2.transfer() to drain the current balance of that token.

Since most AMM on-chain have liquidity pool tokens these tokens may be stole or otherwise used by an attacker by exploiting the LibSwap.swap() function.

Proof of Concept

        (bool success, bytes memory res) = _swapData.callTo.call{ value: msg.value }(_swapData.callData);

Recommended Mitigation Steps

Consider implementing the swap() function for each protocol (e.g. uniswap, sushiswap one inch) individually and then only allow the user to call these functions with specific data.

H3xept commented 2 years ago

We are aware that the contract allows users to use latent funds, although we disagree on it being an issue as no funds (ERC20 or native) should ever lay in the contract. To make sure that no value is ever kept by the diamond, we now provide refunds for outstanding user value (after bridges/swaps).

gzeoneth commented 2 years ago

Duplicate #117