Good Entry - hassan-truscova's results

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/TokenisableRange.sol#L222-L285 https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/TokenisableRange.sol#L240

Vulnerability details

Disclaimer

I am aware that the FullMath and LiquidityAmounts libraries are not in scope. However, i saw this edge-case and thought i should at least inform.

Impact

The deposit function when queries Uniswap pool for sqrtPriceX96, it receives a 160-bit unsigned integer value. The maximum value that can be represented in 160-bits is "1461501637330902918203684832716283019655932542975". Now, when sqrtPriceX96 goes into getAmountsForLiquidity(...) function, the transaction reverts because of a desired intermediate overflow, which never happens due to solidity 0.8.4.

Below is the code snippet for deposit

(uint160 sqrtPriceX96,,,,,,)  = IUniswapV3Pool(pool).slot0();
  (uint256 token0Amount, uint256 token1Amount) = LiquidityAmounts.getAmountsForLiquidity( sqrtPriceX96, TickMath.getSqrtRatioAtTick(lowerTick), TickMath.getSqrtRatioAtTick(upperTick), liquidity);

the function getAmountsForLiquidity(...) further eventually calls FullMath lib.

 FullMath.mulDiv(
            uint256(liquidity) << FixedPoint96.RESOLUTION,
            sqrtRatioBX96 - sqrtRatioAX96,
            sqrtRatioBX96
        ) / sqrtRatioAX96;

That's where the problem starts. The FullMath library was taken from Uniswap v3-core. However, the original solidity version that was used was < 0.8.0, meaning that the execution didn't revert when an overflow was reached. This effectively means that when a phantom overflow (a multiplication and division where an intermediate value overflows 256 bits) occurs the execution will revert and the correct result won't be returned. The original library was designed in a way that could handle intermediate overflows.

The correct result isn't returned in this case and the execution gets reverted when a phantom overflows occurs because the solidity version is "pragma solidity ^0.8.4;".

Hence, The FullMath contract doesn't correctly handle the case when an intermediate value overflows 256 bits. This happens because an overflow is desired in this case but it's never reached.

Proof of Concept

As a proof of concept, you can input the maximum value that can fit in 160-bits to the getAmountsForLiquidity function.

Tools Used

Manual review + in-house tool

Recommended Mitigation Steps

The fix is to mark the full body in an unchecked block, in order to leverage the fact that the original version was designed in order to allow the intermediate overflow. A modified version of the original Fullmath library that uses unchecked blocks to handle the intended overflow, can be found in the 0.8 branch of the Uniswap v3-core repo.

Assessed type

Under/Overflow