code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

When depositing users can supply either/or n0 && n1 which represents the quote and base asset. When the current tick is within the tick ranges for the respective position, the user may receive both n0 & n1 even when depositing only one of the two assets.

The accounting in deposit doesn't consider this case and will cause the function to revert due to underflow, denying a user the ability to deposit assets unilaterally.

Proof of Concept

First to understand this case in UniswapV3Pool specifically:

amount0 = SqrtPriceMath.getAmount0Delta(
                    _slot0.sqrtPriceX96,
                    TickMath.getSqrtRatioAtTick(params.tickUpper),
                    params.liquidityDelta
                );
                amount1 = SqrtPriceMath.getAmount1Delta(
                    TickMath.getSqrtRatioAtTick(params.tickLower),
                    _slot0.sqrtPriceX96,
                    params.liquidityDelta
                );

Here we can see the user is able to receive both quote & base.

In TokenisableRange#deposit these values are stored here:

(uint128 newLiquidity, uint256 added0, uint256 added1) = POS_MGR
            .increaseLiquidity(
                INonfungiblePositionManager.IncreaseLiquidityParams({
                    tokenId: tokenId,
                    amount0Desired: n0,
                    amount1Desired: n1,
                    amount0Min: (n0 * 95) / 100,
                    amount1Min: (n1 * 95) / 100,
                    deadline: block.timestamp
                })
            );

However the issue is caused here, where for example:

User deposits 0 n0 and receives X added0 due to depositing Y n1 & receives Z added1

This will cause the function to revert due to the following check.

TOKEN0.token.safeTransfer(msg.sender, n0 - added0);
TOKEN1.token.safeTransfer(msg.sender, n1 - added1);

Tools Used

manual

Recommended Mitigation Steps

adjust accounting to consider case where user deposits 0 n0/n1

Assessed type

Math

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #414

c4-judge commented 1 year ago

gzeon-c4 changed the severity to QA (Quality Assurance)