Lines of code

Vulnerability details

Impact

No validation on poolIdx input for key functions like claimConcentratedRewards. Could pass invalid poolId and corrupt storage.

The claimConcentratedRewards function is defined on LiquidityMining.sol.

It takes in a poolIdx as one of the parameters

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

  // Reward claiming logic  

}

However, there is no validation on the poolIdx input anywhere in the function.

This could allow an invalid poolId to be passed in, resulting in errors or storage corruption. For example:

claimConcentratedRewards(
  owner, 
  0x123456789, // Invalid poolId
  -60, 
  60,
  [1] 
);

Proof of Concept

function claimConcentratedRewards:
<https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L156-L196&gt;

function claimConcentratedRewards(
  address payable owner,
  bytes32 poolIdx, // Pool identifier 
  // other params
) internal {

  // Claim reward logic

}

The problem is there is no validation on the poolIdx input anywhere in the function.

This means any arbitrary 32 byte value can be passed in as the pool identifier. For example:

claimConcentratedRewards(
  owner,
  0x123456789012345678901234567890123456, // Invalid poolId
  // other params  
);

Passing an invalid poolId like this can corrupt storage by:

• Writing reward tracking data to invalid array indexes
• Accruing time-weighted liquidity for non-existent pools
• Transferring funds to the wrong accounts

It violates the assumption that poolIdx maps to a valid pool.

Here is some example showing how an attacker could pass an invalid poolID to corrupt storage in claimConcentratedRewards():

contract Attacker {

  LiquidityMining public liquidityMining;

  // Construct with real liquidity mining address
  constructor(address _liquidityMiningAddress) {
    liquidityMining = LiquidityMining(_liquidityMiningAddress);
  }

  function exploitInvalidPoolId() public {
    bytes32 invalidPoolId = 0x123456789012345678901234567890123456;

    liquidityMining.claimConcentratedRewards{value: 0}(
      address(1),
      invalidPoolId,
      -60,
      60,
      [uint32(block.timestamp)]
    );
  }

}

This attacker contract calls claimConcentratedRewards() with an invalid poolID that does not map to any real pool.

Since there is no validation, this invalid ID will be used in the reward logic, corrupting storage:

• Invalid array indexes accessed in tickTracking_
• Time-weighted liquidity accrued for fake pool
• concLiquidityRewardsClaimed_ modified for non-existent pool

Over time this could seriously corrupt storage and accounting for the protocol.

Tools Used

Manual

Recommended Mitigation Steps

Adding a require statement like:

require(isValidPool(poolIdx), "Invalid pool");

Would prevent invalid poolIds from being used.

The lack of validation on this input parameter is a security risk.

Assessed type

Invalid Validation

---

The text was updated successfully, but these errors were encountered:

All reactions