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

3 stars 2 forks source link

slot0 is easily manipulatable #578

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/TokenisableRange.sol#L222 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/TokenisableRange.sol#L239

Vulnerability details

Impact

The deposit amount of a user can be manipulated.

Proof of Concept

slot0() is extremely easy to manipulate as it is the most recent data point. The issue arises due to there not being any protection against sqrtPriceX96 manipulation.

// @audit no check against sqrtPriceX96
(uint160 sqrtPriceX96,,,,,,)  = IUniswapV3Pool(pool).slot0();
(uint256 token0Amount, uint256 token1Amount) = LiquidityAmounts.getAmountsForLiquidity( sqrtPriceX96, TickMath.getSqrtRatioAtTick(lowerTick), TickMath.getSqrtRatioAtTick(upperTick), liquidity);
if (token0Amount + fee0 > 0) newFee0 = n0 * fee0 / (token0Amount + fee0);
if (token1Amount + fee1 > 0) newFee1 = n1 * fee1 / (token1Amount + fee1);
fee0 += newFee0;
fee1 += newFee1; 
n0   -= newFee0;
n1   -= newFee1;

https://docs.uniswap.org/contracts/v3/reference/core/interfaces/pool/IUniswapV3PoolState#slot0

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using TWAP to make the calculation.

Assessed type

Uniswap

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #48

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Invalid