code-423n4 / 2023-10-canto-findings

0 stars 1 forks source link

LiquidityMining.claimConcentratedRewards() does not properly account user liquidity across ticks #209

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/marjon-call/2023-10-canto/blob/29c92a926453a49c8935025a4d3de449150fc2ff/canto_ambient/contracts/mixins/LiquidityMining.sol#L156-L196

Vulnerability details

Let’s say a user creates two separate positions, one is [tick-15, tick] and the second is [tick, tick+15]. The user is covering the entirety of the tick range to receive rewards but does not receive any.

We see that posKey is defined like this:

bytes32 posKey = encodePosKey(owner, poolIdx, lowerTick, upperTick);

This uses the lowerTick and upperTick of a position to create posKey. Then liquidity is calculated based on using posKey:

inRangeLiquidityOfPosition += timeWeightedWeeklyPositionInRangeConcLiquidity_[poolIdx][posKey][week][j];

Recommendation: The current system checks a range of ticks. The way this range is checked is based on if the user provided liquidity in the specific range. Instead, check if each tick has been provided liquidity. This also saves a user from having to do an extra claimConcentratedRewards() if they have multiple in range positions that have different start and end indexes.

Assessed type

Math

c4-pre-sort commented 1 year ago

141345 marked the issue as low quality report

141345 commented 1 year ago

position_1 + position_2 not combined automatically

expected behavior user's own choice

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Invalid