Liquidity providers may recieve wrong rewards due to loss of precision in the calculation of `currWeek` and `nextWeek`.

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/37a1d64cf3a10bf37cbc287a22e8991f04298fa0/canto_ambient/contracts/mixins/LiquidityMining.sol#L51-L52 https://github.com/code-423n4/2023-10-canto/blob/37a1d64cf3a10bf37cbc287a22e8991f04298fa0/canto_ambient/contracts/mixins/LiquidityMining.sol#L96-L97 https://github.com/code-423n4/2023-10-canto/blob/37a1d64cf3a10bf37cbc287a22e8991f04298fa0/canto_ambient/contracts/mixins/LiquidityMining.sol#L208-L209 https://github.com/code-423n4/2023-10-canto/blob/37a1d64cf3a10bf37cbc287a22e8991f04298fa0/canto_ambient/contracts/mixins/LiquidityMining.sol#L238-L239

Vulnerability details

Throughout LiquidityMining.sol the values for currWeek and nextWeek are generated using the lastAccrued timestamp embedded in a local variable time.
currWeek is determined by

uint32 currWeek = uint32((time / WEEK) * WEEK);

And nextWeek is calculated by

 uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);

It can be clearly seen that division before multiplication is implemented to derive these values which is prone to precision loss, for example in conditions where time is divided by WEEK there will be a huge loss of precision in the value as roundings could occur to the value before be multiplied, you can see that dt is later multiplied with liquidity and accrued to timeWeightedWeeklyPositionAmbLiquidity_ which is later used to calculate the rewardForWeek

FIle: canto_ambient/contracts/mixins/LiquidityMining.sol

273:            uint256 overallTimeWeightedLiquidity = timeWeightedWeeklyGlobalAmbLiquidity_[
274:                    poolIdx
275;                ][week]

277:                uint256 rewardsForWeek = (timeWeightedWeeklyPositionAmbLiquidity_[
278:                    poolIdx
278:               ][posKey][week] * ambRewardPerWeek_[poolIdx][week]) /
280:                    overallTimeWeightedLiquidity;
281:               rewardsToSend += rewardsForWeek;

for each week that the user claims, when the values of (timeWeightedWeeklyPositionAmbLiquidity_[poolIdx][posKey][week] x ambRewardPerWeek_[poolIdx][week]) is divided by a lower timeWeightedWeeklyPositionAmbLiquidity_ the rewards the user gets will be higher.

There are different Instances of this multiplication on the result of a division which can lead to users claiming incorrect amount of rewards.

• timeWeightedWeeklyGlobalConcLiquidity_
• timeWeightedWeeklyPositionInRangeConcLiquidity_
• timeWeightedWeeklyGlobalAmbLiquidity_
• timeWeightedWeeklyPositionAmbLiquidity_

  Impact

  Wrong values for these variables are generated, which in turn when used to calculate the rewards for a liquidity provider will result in an incorrect amount of rewards given to LPs. This will cause some LPs to get less amount of rewards, and some get more amount of rewards than supposed to.

Proof of Concept

• Whenever currWeek and nextWeek are generated
• Division before multiplication is used to calculate its values
• In a situation where rounding after division occurs.
• Wrong values are generated which are then used to calculate incorrect amounts to be given to LPs when the claim rewards within the protocol.

Tools Used

Manual Review

Recommended Mitigation Steps

The use of multiplication before division is advised as it is less prone to precision loss, Multiply before you divide or use safeMath library.

Assessed type

Math

[c4-pre-sort]

141345 marked the issue as duplicate of #18

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[c4-judge]

dmvt marked the issue as satisfactory

[c4-judge]

dmvt marked the issue as unsatisfactory: Invalid