Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LPholders can be prevented from burning their position NFTs #64

Closed
howlbot-integration bot opened this issue Nov 4, 2024 · 1 comment
Closed
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-21 🤖_07_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/ronin-chain/katana-v3-contracts/blob/03c80179e04f40d96f06c451ea494bb18f2a58fc/src/periphery/NonfungiblePositionManager.sol#L246
https://github.com/ronin-chain/katana-v3-contracts/blob/03c80179e04f40d96f06c451ea494bb18f2a58fc/src/periphery/NonfungiblePositionManager.sol#L410

Vulnerability details

Proof of Concept

Liquidity providers can burn their position during NonfungiblePositionManager.sol:collect() if there's no liquidity and no tokens owed

 function collect(CollectParams calldata params)
    external
    payable
    override
    isAuthorizedForToken(params.tokenId)
    returns (uint256 amount0, uint256 amount1)
  {
    .......

    // if there's no liquidity and no tokens owed, burn the position
 
 @>   if (position.liquidity == 0) {
      delete _positions[params.tokenId];
 @>     _burn(params.tokenId);
    }

....
  }

But increaseLiquidity has no access control, and anyone can increase a position's liquidity by any amount, no matter how little.

    function increaseLiquidity(
        IncreaseLiquidityParams calldata params
    )
        external
        payable
        override
        checkDeadline(params.deadline)
        returns (uint128 liquidity, uint256 amount0, uint256 amount1)
    {
  @>      Position storage position = _positions[params.tokenId];

        PoolAddress.PoolKey memory poolKey = _poolIdToPoolKey[position.poolId];

        IRamsesV3Pool pool;
  @>      (liquidity, amount0, amount1, pool) = addLiquidity(
            AddLiquidityParams({
                token0: poolKey.token0,
                token1: poolKey.token1,
                tickSpacing: poolKey.tickSpacing,
                tickLower: position.tickLower,
                tickUpper: position.tickUpper,
                amount0Desired: params.amount0Desired,
                amount1Desired: params.amount1Desired,
                amount0Min: params.amount0Min,
                amount1Min: params.amount1Min,
                recipient: address(this),
                index: params.tokenId
            })
        );

// ...
 @>      position.liquidity += liquidity;

A malicious user in a griefing attack can continously increase liquduity by as low as 1 wei to continously dos attempt to burn a position

Recommended Mitigation Steps

Ensure only a position owner can increase his liquidity.

Assessed type

DoS

@howlbot-integration howlbot-integration bot added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 🤖_07_group AI based duplicate group recommendation bug Something isn't working duplicate-21 sufficient quality report This report is of sufficient quality labels Nov 4, 2024
howlbot-integration bot added a commit that referenced this issue Nov 4, 2024
@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Nov 11, 2024
@c4-judge
Copy link

alex-ppg marked the issue as unsatisfactory:
Invalid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-21 🤖_07_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

1 participant