Platform: Code4rena
Start Date: 01/08/2023
Pot Size: $91,500 USDC
Total HM: 14
Participants: 80
Period: 6 days
Judge: gzeon
Total Solo HM: 6
Id: 269
League: ETH
Rank: 25/80
Findings: 1
Award: $482.48
🌟 Selected for report: 0
🚀 Solo Findings: 0
482.4792 USDC - $482.48
The function getTickAtSqrtRatio is used multiple times in the TokenisableRange.sol, but the library TickMath.sol is compiled with pragma solidity ^0.8.4 as you can see here
https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/lib/TickMath.sol#L2
which doesn't allow for overflows, and since the function is not unchecked, getTickAtSqrtRatio will not behave as intended since it relies implicitly on overflows.
Here is the UniswapV3 version of the function https://github.com/Uniswap/v3-core/blob/d8b1c635c275d2a9450bd6a78f3fa2484fef73eb/contracts/libraries/TickMath.sol#L61-L204 and here is the Good Entry version https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/lib/TickMath.sol#L93-L253 As you can see they are identical, but the Good entry version will not let overflows happens, every time this function is used in the code it could revert and break the functionality.
Manual review
Apply the same strategy as UniswapV3 used in their 0.8.0 versions of their libraries by putting the whole function into an unchecked block as you can see here https://github.com/Uniswap/v3-core/blob/6562c52e8f75f0c10f9deaf44861847585fc8129/contracts/libraries/TickMath.sol#L68-L213
Uniswap
#0 - c4-pre-sort
2023-08-10T06:05:37Z
141345 marked the issue as duplicate of #58
#1 - 141345
2023-08-10T06:09:19Z
different library, but similar issue https://github.com/code-423n4/2023-08-goodentry-findings/issues/58
this report:
File: contracts\TokenisableRange.sol 99: int24 _upperTick = TickMath.getTickAtSqrtRatio( uint160( 2**48 * sqrt( (2 ** 96 * (10 ** TOKEN1.decimals)) * 1e10 / (uint256(startX10) * 10 ** TOKEN0.decimals) ) ) );
https://github.com/code-423n4/2023-08-goodentry-findings/issues/58:
File: contracts\TokenisableRange.sol 338: (amt0, amt1) = LiquidityAmounts.getAmountsForLiquidity( uint160( sqrt( (2 ** 192 * ((TOKEN0_PRICE * 10 ** TOKEN1.decimals) / TOKEN1_PRICE)) / ( 10 ** TOKEN0.decimals ) ) ), TickMath.getSqrtRatioAtTick(lowerTick), TickMath.getSqrtRatioAtTick(upperTick), liquidity);
#2 - c4-judge
2023-08-20T17:30:19Z
gzeon-c4 changed the severity to 3 (High Risk)
#3 - c4-judge
2023-08-20T17:30:25Z
gzeon-c4 marked the issue as satisfactory
482.4792 USDC - $482.48
https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/TokenisableRange.sol#L338 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/TokenisableRange.sol#L371 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/TokenisableRange.sol#L240
getAmountsForLiquidity which is used in TokenisableRange.sol has the mulDiv function which is taken from UniswapV3 FullMath library, function which require overflow behavior, but that behavior will not be allowed in the Good Entry TokenisableRange.sol contract, which would make the getTokenAmountsExcludingFees , returnExpectedBalanceWithoutFees and deposit revert most of the time.
The contract LiquidityAmounts.sol uses solidity version >=0.5.0 as can be seen here https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/lib/LiquidityAmounts.sol#L2
which will allow for overflows, but FullMath.sol and TokenisableRange.sol is compiled with version of pragma solidity bigger than 0.8.0 as can be seen here
https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/lib/FullMath.sol#L2
https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/TokenisableRange.sol#L2
which will also compile LiquidityAmounts.sol with the version ^0.8.0 since that's how inheritance works in solidity, making the mulDiv function reverting all the time on overflows.
Manual review
Use uncheck boxes on the mulDiv function so the overflows would still happen
Uniswap
#0 - c4-pre-sort
2023-08-10T07:25:33Z
141345 marked the issue as duplicate of #58
#1 - c4-judge
2023-08-20T17:30:19Z
gzeon-c4 changed the severity to 3 (High Risk)
#2 - c4-judge
2023-08-20T17:30:25Z
gzeon-c4 marked the issue as satisfactory