Unvalidated ticks in `claimConcentratedRewards` allow unauthorized users to claim undeserved rewards. Validate ticks.

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L156-L168

Vulnerability details

Impact

There is no check that the ticks passed into claimConcentratedRewards actually match the position's ticks. A user could pass in arbitrary ticks to try to claim rewards for liquidity they don't own.

Proof of Concept

The claimConcentratedRewards function allows users to claim mining rewards for a specific position they hold, identified by the lowerTick and upperTick range.

Here's the vulnerability related to validating the tick inputs in the claimConcentratedRewards function.

function claimConcentratedRewards(
    address payable owner, 
    bytes32 poolIdx,
    int24 lowerTick,
    int24 upperTick,
    uint32[] memory weeksToClaim
) internal {

  // Lookup position based on passed ticks
  RangePosition72 storage pos = lookupPosition(
      owner, 
      poolIdx,
      lowerTick,  
      upperTick
  );

  // Rest of reward calculation and transfer

The lowerTick and upperTick passed into the function are used to look up the position via lookupPosition.

However, there is no validation that these match the actual ticks of the position stored in the contract's mapping.

This means a malicious user could pass in arbitrary tick values that don't match their position, and claim rewards for liquidity they don't actually own.

To clarify, here's a simplified proof of concept demonstrating the lack of tick validation:

// Simplified contract with the same vulnerability
contract LiquidityMining {
    struct RangePosition72 {
        int24 lowerTick;
        int24 upperTick;
    }

    mapping(address => mapping(bytes32 => RangePosition72)) positions;

    function claimConcentratedRewards(
        address payable owner, 
        bytes32 poolIdx,
        int24 lowerTick,
        int24 upperTick,
        uint32[] memory weeksToClaim
    ) internal {
        // Lookup position based on passed ticks
        RangePosition72 storage pos = positions[owner][poolIdx];

        // Rest of reward calculation and transfer
    }
}

The claimConcentratedRewards function retrieves the position based on the passed lowerTick and upperTick without validating whether these ticks match the position's ticks stored in the mapping of the position.

To exploit this, a user could call claimConcentratedRewards with arbitrary lowerTick and upperTick values, potentially claiming rewards for liquidity they don't own. The contract lacks the validation checks to ensure that the provided ticks match the actual position.

Tools Used

Vs

Recommended Mitigation Steps

The ticks should be validated against the position, the contract should validate lowerTick and upperTick match the position's ticks after looking it up:

// Lookup position 
RangePosition72 storage pos = lookupPosition(/*...*/);

// Validate ticks match position  
require(pos.lowerTick == lowerTick, "Invalid lower tick");
require(pos.upperTick == upperTick, "Invalid upper tick");

// Rest of function

Adding this tick validation prevents the attack vector where users can claim arbitrary liquidity rewards.

Assessed type

Invalid Validation

[c4-pre-sort]

141345 marked the issue as low quality report

[c4-judge]

dmvt marked the issue as unsatisfactory: Insufficient quality