Global and Position liquidity accrual can significantly impact the performance

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/main/canto_ambient/contracts/mixins/LiquidityMining.sol#L51-L52

Vulnerability details

Impact

Global and Position liquidity accrual can significantly impact the performance

Proof of Concept

The calculation of currWeek and nextWeek as shown in the provided code snippet aims to determine two time points within a week, primarily for the purpose of tracking liquidity accrual.

   function accrueConcentratedGlobalTimeWeightedLiquidity(
        bytes32 poolIdx,
        CurveMath.CurveState memory curve
    ) internal {
        uint32 lastAccrued = timeWeightedWeeklyGlobalConcLiquidityLastSet_[
            poolIdx
        ];
        // Only set time on first call
        if (lastAccrued != 0) {
            uint256 liquidity = curve.concLiq_;
            uint32 time = lastAccrued;
            while (time < block.timestamp) {
                uint32 currWeek = uint32((time / WEEK) * WEEK);
                uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);
                uint32 dt = uint32(
                    nextWeek < block.timestamp
                        ? nextWeek - time
                        : block.timestamp - time
                );
                timeWeightedWeeklyGlobalConcLiquidity_[poolIdx][currWeek] += dt * liquidity;
                time += dt;
            }
        }
        timeWeightedWeeklyGlobalConcLiquidityLastSet_[poolIdx] = uint32(
            block.timestamp
        );
    }

Here's a detailed explanation of the problem, along with a breakdown in points:

uint32 currWeek is intended to represent the start of the current week uint32 nextWeek is intended to represent the start of the next week and WEEK is defined as the duration of one week in seconds currWeek is calculated by dividing time by WEEK (the duration of a week in seconds) This calculation effectively rounds down time to the nearest multiple of WEEK, which gives the start time of the current week.

nextWeek is calculated by adding WEEK to time, then dividing the result by WEEK, This calculation determines the start time of the week immediately following the current week. the problem with the above mentioned code comes near End of Current Week:

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

Suppose their is a scenario when the current week is just about to end In this scenario, currWeek correctly represents the start of the current week. However, nextWeek is calculated as if time were already in the next week, resulting in an inaccurate representation of the start of the following week.

And Since both currWeek and nextWeek may effectively point to the same week when time is near the end of the current week, it can lead to incorrect calculations for liquidity accrual.

Tools Used

manual

Recommended Mitigation Steps

update nextWeek as uint32 nextWeek = currWeek + uint32(WEEK); in https://github.com/code-423n4/2023-10-canto/blob/main/canto_ambient/contracts/mixins/LiquidityMining.sol#L52

Assessed type

Context

[141345]

invalid

+ WEEK will guarantee to be bigger

[c4-pre-sort]

141345 marked the issue as low quality report

[c4-pre-sort]

141345 marked the issue as primary issue

[c4-judge]

dmvt marked the issue as unsatisfactory: Invalid