code-423n4 / 2024-04-panoptic-findings

7 stars 3 forks source link

In the swapInAMM the malicious actor could manipulate the sqrtPriceX96 in the slot0 which can cause an incorrect calculation. #483

Closed c4-bot-8 closed 4 months ago

c4-bot-8 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/SemiFungiblePositionManager.sol#L787-L845

Vulnerability details

Summary

In the SFPM.sol the swapInAMM function is triggered when the position minted/burned in ITM, and the tokens need to be swapped (because user receive only one token). However the problem is that during the swap calculation the (uint160 sqrtPriceX96, , , , , , ) = _univ3pool.slot0(); is used. Since the sqrtPriceX96 is easily manipulated by the MEV bots / Flashloans the user funds could be lost due to manipulation.

Vulnerability details

Let's desing the flow of the attack.

  1. When the position is minted/burned the _validateAndForwardToAMM function is triggered. In this function the burning/minting mechanism in the UniswapV3 is triggered through the _createLegInAMM function. Eventually, this function gather the itmAmounts based on the amount moved out/from the Uniswap, this itm amount is used further to execute the swap
  2. Then, the condition in _validateAndForwardToAMM is checked, the tickLimitLow should be higher than tickLimintHigh (tickLimitLow > tickLimitHigh). If yes -> swapInAmm function is triggered
  3. In the swapInAmm the itmAmounts are retrieved and if both of them > than 0, the swap occured.

The exact problem here is that during the swap the (uint160 sqrtPriceX96, , , , , , ) = _univ3pool.slot0(); is taken which could cause the swap to not work as intended.

negative ITM amounts denote a surplus of tokens (burning liquidity), while positive amounts denote a shortage of tokens (minting liquidity)

int256 net0 = itm0 - PanopticMath.convert1to0(itm1, sqrtPriceX96); And we can see that the convert1to0 basically does this --> return Math.mulDiv(amount, 2 192, uint256(sqrtPriceX96) 2);

As we can see here, the malicious user could inflate the sqrtPriceX96, thus the prices will be calculated not correctly and user will suffer from the incorrect conversions.

Tools Used

Manual Review

Recommended Mitigation Steps

Since it is a well know vulnerability that is was previosly found many times, i suggest to use the TWAP prices instead: https://solodit.xyz/issues/h-02-use-of-slot0-to-get-sqrtpricelimitx96-can-lead-to-price-manipulation-code4rena-maia-dao-ecosystem-maia-dao-ecosystem-git

Assessed type

Uniswap

c4-judge commented 4 months ago

Picodes marked the issue as duplicate of #562

c4-judge commented 4 months ago

Picodes marked the issue as unsatisfactory: Insufficient proof

Picodes commented 4 months ago

There are slippage limits in place to prevent pool manipulation

c4-judge commented 4 months ago

Picodes marked the issue as unsatisfactory: Invalid