code-423n4 / 2023-08-dopex-findings

3 stars 3 forks source link

No check for minimum amount in the UniswapV2/V3 functions lead to slippage. #1828

Closed code423n4 closed 11 months ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L189-L194 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L258-L262 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L304-L307 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L155-L157 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L213-L217 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L274-L281

Vulnerability details

Impact

The minimum amount in the AMMs functions is not checked and this can lead to receive tokens or mint at an inaccurate price.

Proof of Concept

https://docs.uniswap.org/contracts/v3/guides/providing-liquidity/mint-a-position https://docs.uniswap.org/contracts/v3/guides/swaps/single-swaps

Take a look in the UniswapV3 documentation, with the minting example:

"We set amount0Min and amount1Min to zero for the example - but this would be a vulnerability in production. A function calling mint with no slippage protection would be vulnerable to a frontrunning attack designed to execute the mint call at an inaccurate price."

And take a look in the swap example also in UniswapV3 documentation:

"amountOutMinimum: we are setting to zero, but this is a significant risk in production. For a real deployment, this value should be calculated using our SDK or an onchain price oracle - this helps protect against getting an unusually bad price for a trade due to a front running sandwich or another type of price manipulation"

For example, if a malicious user wants to manipulate the price, he can check when you send a transaction, frontrun it with a large liquidity movement in the rdpx/weth Uniswap pool, and because these functions doesn't have sufficient checks, you can lose values in the transaction.

UniV2LiquidityAmo

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L189-L194

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L258-L262

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L304-L307

UniV3LiquidityAmo

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L155-L157

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L213-L217

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L274-L281

Tools Used

Manual Review

Recommended Mitigation Steps

I recommend to add a slippage control for all the Uniswap functions.

Assessed type

Uniswap

bytes032 commented 11 months ago

Invalid

c4-pre-sort commented 11 months ago

bytes032 marked the issue as low quality report

c4-judge commented 11 months ago

GalloDaSballo marked the issue as unsatisfactory: Invalid