An attacker can exploit the accruing liquidity functionality to accrue liquidity for more weeks than intended.

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L50-L64 https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L237-L253 https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L207-L221 https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L94-L153

Vulnerability details

Instances

The whole exploit works due to similar functionality being broken at these 4 instances: here, here, here and here.

Impact

An attacker can accrue both Position time weighted liquidity and Global time weighted liquidity for both concentrated and ambient for more weeks than intended by the protocol. This results in attacker being able to claim rewards for more weeks than intended resulting in severe loss of funds to the protocol and hence the high severity.

This issue occurs because of incorrectly updating block.timestamp on first liquidity accrue.

Proof of Concept

Please refer to the below PoC which uses accrueConcentratedGlobalTimeWeightedLiquidity() function to demonstrate how an attacker can claim rewards for two weeks than one which is intended:

https://gist.github.com/0xSandyy/8e4d325e6da65554891b69fa86fb9481

The attack vector with the help of this PoC is explained below:

1. The main issue is with block.timestamp getting incorrectly updated on the first accrue as it is updated directly to the actual value here instead of being rounded down to nearest week like currWeek and nextWeek is done here.

2. If the attacker calls claimConcentratedRewards() in around one week after previous rewards claim, the timeWeightedWeeklyGlobalConcLiquidityLastSet_ mapping is updated for two weeks instead of one as only one week has passed and this can be proven by the test_accrueIterationMoreDueToPrecisionLoss() function in this PoC.

NOTE: The claimConcentratedRewards() don't need to be called in exact 604800 seconds ( 1 week) and this can be tested in test_pocStillWorksWithoutExactOneWeek() function the PoC.

3. As all the four instances, here, here, here and here have the same functionality, all the respective mappings will be updated for two weeks instead of one and as these mappings are directly used in reward calculation eg:here and here, the reward for two weeks can be claimed by the attacker instead of one week and thus protocol will lose funds.

To set up the test file in the cloned repository run the following commands:

forge init --force

forge install openzeppelin/openzeppelin-contracts@v4.7.0 --no-commit

forge fmt

Tools Used

Manual Analysis

Recommended Mitigation Steps

To mitigate this issue, this block.timestamp on first call should be updated like this on all the four instances here, here, here and here:

./LiquidityMining.sol

        timeWeightedWeeklyGlobalConcLiquidityLastSet_[poolIdx] = uint32(
            (block.timestamp / WEEK) * WEEK)
        );

The test_accrueIterationsMoreDueToPrecisionLossFixed() function of this PoC proves that the above fix works.

Assessed type

Other

[141345]

accrue time should round by WEEK

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[OpenCoreCH]

Thanks for including a PoC. However, I do not see the actual issue here. When someone joins in the middle of week X and leaves in week X + 1, it is intended that they can claim for week X and X + 1. However, the amount will be partial (based on dt, which is the time-multiplier for the liquidity). I also logged dt in the provided PoC:

The value of currWeek for iteration no.1 is 1695859200 The value of nextWeek for iteration no.1 is 1696464000 455946 The value of currWeek for iteration no.2 is 1696464000 The value of nextWeek for iteration no.2 is 1697068800 148854 The iteration lasts 2 times.

As we can see in this output, dt is not equal to WEEK, so it will only accrue partial rewards for both weeks.

[c4-sponsor]

OpenCoreCH (sponsor) disputed

[c4-judge]

dmvt marked the issue as unsatisfactory: Invalid