Canto Liquidity Mining Protocol - Banditx0x's results

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L139-L153

Vulnerability details

Impact

Attempts to mint, burn and harvest positions will fail if the concentrated position is across many ticks. This is caused by calls to accrueConcentratedGlobalTimeWeightedLiquidity will always revert as it will exceed the gas limit.

Proof of Concept

The accrue function iterates through every tick in a concentrated liquidity position aside from the first 10 ticks and the last 10 ticks. However, according to current intended configurations of 1bps ticks, a full range tick position will go from min tick of -887272 to max tick of 887272. There is no filter which ensures that this loop iteration is only performed on positions with a small number of ticks.

Performing an iteration loop (887272 * 2 - 20) times will inevitably lead to an 'out-of-gas' error.

There are elements in the loop that will consume gas:

1. The loop iteration itself.
2. Reading from storage (tickTracking_[poolIdx][i].length, tickTracking_[poolIdx][i][numTickTracking - 1].exitTimestamp).
3. Writing to storage (tickTrackingIndexAccruedUpTo_[poolIdx][posKey][i]).

Users often put such wide liquidity widths in concentrated liquidity DEX's such as Uniswap v3 and Ambient. However, ever much more narrower positions have many ticks because each tick is only 1 bps wide.

Tools Used

Manual Review

Recommended Mitigation Steps

The rewards program was initially designed for stable pools. Incentives could be targeted to a liquidity range that assumes that the assets will stay pegged to the stable price during the week. Instead of iterating through every tick, the depth of liquidity can be calculated by the formula:

liquidity * time /(upper_tick - lower_tick)

upper_tick - lower_tick returns the "width" of the liquidity position and is hence inversely proportional to the concentration. Dividing by this number means that the CANTO rewards distribution are adjusted for the concentration and hence only the in-range liquidity.

Assessed type

Loop

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L24-L35 https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L122

Vulnerability details

Impact

A malicious user can brick minting, burning and harvesting of liquidity for almost all liquidity providers.

Important NOTE: This is a different vector from another gas issue, which is iterating over too many ticks in (int24 i = lowerTick + 10; i <= upperTick - 10; ++i). That issue affects wide liquidity positions, while this attack vector affects even liquidity positions with a relatively small number of ticks.

Proof of Concept

When accrueConcentratedPositionTimeWeightedLiquidity is called, under most conditions, for every potentially eligible tick, it will iterate over every tickTrackingData in tickTracking:

while (time < block.timestamp && tickTrackingIndex < numTickTracking)

tickTracking is iterated by tickTrackingIndex++;

The array mapped by tickTracking_ is increased by 1 for a tick every time a trade through the liquidity pool changes the price from a different tick to this tick. This is implemented in the crossTicks function:

    function crossTicks(
        bytes32 poolIdx,
        int24 exitTick,
        int24 entryTick
    ) internal {
        uint256 numElementsExit = tickTracking_[poolIdx][exitTick].length;
        tickTracking_[poolIdx][exitTick][numElementsExit - 1]
            .exitTimestamp = uint32(block.timestamp);
        StorageLayout.TickTracking memory tickTrackingData = StorageLayout
            .TickTracking(uint32(block.timestamp), 0);
        tickTracking_[poolIdx][entryTick].push(tickTrackingData);
    }

A user could purposely increase the length of the tickTracking_ array and hence cause the gas limit to be reached whenever the array is looped over.

The price impact required to cross a tick is from 0 to 1 bps, as 1 bps as the tick width. This is already extremely small, but the attacker could have the swap amount be a very small fraction of a bps if they first swap to make the price end very close to a tick boundary, and then execute multiple extremely small swaps which bounce the price back and forth over the tick boundary.

Note that the CANTO liquidity rewards are targeted to stable pools. An attacker can be quite confident, for example, that a USDC/USDT pool will trade at around $1, and the ticks closest to $1 will always be eligible for rewards and therefore be looped over by all rewardable positions when accrueConcentratedPositionTimeWeightedLiquidity is called. Therefore the attack can be targeted to just one or two ticks to affect almost every user.

accrueConcentratedPositionTimeWeightedLiquidity is called during minting, burning and harvesting liquidity positions. Therefore this gas griefing attack will make all these functions revert, for almost every user. This would basically break the functionality of concentrated liquidity pools on Ambient.

Contrast the effect to the cost to the attacker: using the aforementioned attack vector the main cost to the attacker will be the gas costs of performing the swaps. This is far lower than the damage that is done to the protocol/users

One additional factor which makes this attack easy to execute that crossing ticks even if the entry and exit is within the same block.timestamp adds to the array length. Tracking this is unnecessary, because the tick was active for 0 blocks, and therefore the time delta and hence allocated rewards is zero.

Tools Used

Manual Review

Recommended Mitigation Steps

One immediate step would to pop() tickTrackingData as soon as the exitTimestamp == entryTimestamp. This happens to the last element of the array when crossTicks is called. Tracking this is unnecessary, because the tick was active for 0 blocks, and therefore the time delta and hence allocated rewards is zero.

The documentation stated that CANTO rewards are meant to be distributed for stable pools for this codebase. The term "stable" could have different interpretations, but this reccomendation assumes that this refers to stablecoin-like or pegged asset pairs such as stETH/WETH, USDT/USDC etc.

Instead of iterating through every tick, one could assume a range where the stable assets could lie and then reward all positions that lie within the specified range - in this case +- 10 ticks of the price tick.

This makes an assumption that these "stable assets" will actually stay pegged to each other. However, the current accounting architecture has multiple problems:

• Given the high number of loops required by the current accounting mechanism, there are multiple reasons that gas could run out. This includes iterating through too many ticks or having too many tick entries/exits

• The current mechanism increases the gas costs of all minting, burning and harvesting

• DOS attacks like the one described in this issue are possible.

Assuming a stable price has the downside of misallocating rewards if the stable assets depeg from each other. However, this may be a reasonable tradeoff to prevent this DOS attack.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L88-L137

Vulnerability details

Impact

Users get rewarded even for liquidity that is outside of the specified reward ticks, resulting in massive over-allocation of rewards.

This means the reward formula does not fulfil its purpose of incentivising a particular liquidity range/concentration suitable for stable pools. The stable pool reward formula is meant to reward the liquidity concentrated around a narrow within 10 ticks of the stable price, as this is where all the trading is likely to occur.

Proof of Concept

Summary: When concentrated interest is accrued liquidity of a position is calculated by uint256 liquidity = pos.liquidity_; which includes all liquidity, including that which lies outside of the specified reward range. This is in contrast to using uint256 liquidity = curve.concLiq_; which gives the concentrated liquidity in the active tick or adjusting the total liquidity by dividing by the "width" (upperTick - lowerTick) of the position.

According to the specification, the reward program is only supposed to reward the in-range liquidity, within 10 ticks of the current price.

From the ReadMe: "As Canto plans to use LiquidityMining to incentivize stable pools, the range currentTick-10 to currentTick+10 is incentivized."

The reward logic rewards all the liquidity positions that are superset of that range, but also allocates rewards to the liquidity portion outside of that range.

For each position, it iterates through every tick between the lowest 10 and highest 10 ticks:

(int24 i = lowerTick + 10; i <= upperTick - 10; ++i)

For each tick, it checks the tracked tick history. Then, the rewards are added based on the time the tick was active multiplied by the liquidity:

timeWeightedWeeklyPositionInRangeConcLiquidity_[poolIdx][posKey][currWeek][i] +=
                            (tickActiveEnd - tickActiveStart) * liquidity;

The liquidity variable was retrieved from uint256 liquidity = pos.liquidity_;. This liquidity contains sqrt(X*Y) where X and Y are the quantities of the two tokens in the liquidity position. However, there is no adjustment for the liqudity concentration.

Lets say a liquidity position is 1000 ticks wide, and the liqudity supersets the reward range. Only 21 ticks of liquidity is within the reward range, which is ~2% of the entire liqudiity amount. However, since the liquidity is proportional to the enitre pos.liquidity this user gets allocated 49.5x the rewards that they should.

Tools Used

Manual Review

Recommended Mitigation Steps

We can adjust for the concentration of liquidity by multiplying the liquidity by the number of ticks in the reward range divided by the number of ticks in the concentrated liquidity position:

conecntrationAdjustedLiquidity =liquidity * 21 / (upperTick - lowerTick)

The rewards should be proportional to the concentrationAdjustedLiquidity instead of the original liquidity variable.

Assessed type

Math

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L188 https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L48

Vulnerability details

Impact

The rewards are not divided based on the in-range liquidity of eligible positions but also include eligible positions.

The overallInRangeLiquidity and inRangeLiquidityOfPosition have a non-corresponding basis for determining liquidity leading to a miscalculation of rewards

The rewards allocated could exceed the weekly allocated rewards, leading to reward insolvency.

Proof of Concept

When accrueConcentratedGlobalTimeWeightedLiquidity is called, liquidity is determined by:

uint256 liquidity = curve.concLiq_;

The NatSpec comments for the struct CurveState state:

"@param concLiq The total concentrated liquidity active and in range at the current state of the curve."_

Therefore the rewards are divided by the time-aggregated global liquidity, which is the sum of average liquidity active in a tick times the time it was active.

There are 2 problems with this:

Problem 1:

There are non-rewarded positions which are affect the curve.concLiq_; variable, such as liquidity positions that include the active tick but do not superset the tick range which is eligible for rewards. For example, if a user provided a large amount of liquidity concentrated between ticks current_tick - 5 and current_tick + 5, the liqudity would be ineligible for rewards as it does not cover the entire 21 tick range neccessary to qualify for rewards. However, the liquidity will add to uint256 liquidity = curve.concLiq_; which will in turn dilute rewards for current positions.

Problem 2:

The rewards for global liquidity are calculated based on the current tick, while the rewards calculated for individual position liquidity via accrueConcentratedPositionTimeWeightedLiquidity are calculated based on the entire liquidity of a position - uint256 liquidity = pos.liquidity_;, regardless of what tick those positions are. The entire liquidity can deviate massively from a linear correlation to the active liquidity.

For example:

Alice has 50 tokens each of liquidity distributed over 100 ticks. Bob has the same amount of tokens distributed of 10000 ticks. Both of the liquidity ranges cross the "active" liquidity threshold. Even though Bob and Alice have the same liquidity calculated by pos.liquidity used in accrueConcentratedPositionTimeWeightedLiquidity, Alice has 100x the liquidity in the active tick, which is used in curve.concLiq_; in accrueConcentratedGlobalTimeWeightedLiquidity.

When rewards are claimed, the rewards are calculated by WeeklyTotalRewards * InRangeLiquidity / GlobalLiquidityByTime

rewardsToSend += inRangeLiquidityOfPosition * concRewardPerWeek_[poolIdx][week] / overallInRangeLiquidity;

There is an expected invariant that the sum of all inRangeLiquidityOfPosition divided by overallInRangeLiquidity is less than 1. If this invariant is broken, then the aggregate rewards distributed will exceed the total concentrated reward pool per week (concRewardPerWeek_[poolIdx][week]). Since the liquidity basis for individual positions differs from the liquidity calculation basis for the global liquidity, this invariant is broken.

Tools Used

Manual Review

Recommended Mitigation Steps

Three suggested changes:

1. Only eligible liquidity positions should be included for the globalLiquidity used for rewards distribution.

2. The global liquidity calculation should be adjusted to ensure that the global eligible liquidity is greater or equal to the sum of all liquidity positions. Otherwise the rewards allocated would exceed the weekly allocated rewards, leading to reward insolvency.

3. Rewards for individual positions should be adjusted for tick size. pos.liquidity should be divided by upperTick - lowerTick

Assessed type

Math

Contextualising my Findings

A Nuance in Ambient Liquidity Causes Incorrect Reward Accounting - My Favourite Finding

The CANTO scope was short in terms of lines of code but large in terms of background knowledge. The ambient finance contracts are not well known by auditors, as mentioned by the sponsor. When I read into the documentation I noticed a key fact:

"The first advantage is that instead of accumulating fees in a separate side-pocket, fees accumulated by ambient LP positions auto-compound back into the original position without any manual management"

The CANTO rewards system is designed to accrue interest before every change in liquidity amount, such as before minting and burning functions. However, due to fees compounding and increasing the liquidity position the invariant that the liquidity amount between different accrual periods is broken. This is further elucidated in my issue:

"Fee Compounding is Over-rewarded in Ambient Liquidity Positions"

Gas Problems

There are problems with the gas consumption of the contract. The calls to functions like crossTick when a swap is a cost which affects every swap. This cost is imposed every time the price impact causes a price tick is crossed. With a price tick of only 1 bps, this will happen extremely frequently. The first consequence is that the gas for every swap over a liquidity pool is increased slightly. The second consequence is explained in one of my issues:

"Array Length of tickTracking_ Can be Purposely Increased to Brick Minting and Burning of Most Users' Liquidity Positions"

This issue concerns the fact that every time a tick is entered, the array for tickTrading_ increases by one. By repeatedly making small swaps back and forth across a tick, an attacker can cause the array for a particular tick to grow large. This array is iterated over every time other users call mint and burn, and making the array very large will cause these critical functions to revert due to out of gas.

The second seperate issue with gas concerns the iteration over many ticks for "wide" liquidity positions. The program actually loops through every tick in a liquidity position, except the first 10 and last 10. There can be over 160000 ticks in a liquidity position! Iterating over a loop so many times will cause an out of gas error which is my second gas issue:

"Gas Limit Exceeded Due to Iterrating Through Almost Every Tick"

Accounting Issues

There were 2 accounting issues:

"Calculation For Overall Liquidity Includes Ineligible Positions and Does Not Correspond With the Calculation For Individual Liquidity Positions"

This concerns the overallLiquidity tracking including ineligible positions when only eligible positions should be counted towards the global liquidity.

The next issue:

"Concentrated Pool Rewards are Proportional to The Entire Liquidity Amount For Any Eligible Positions, Even if That Liquidity Is Out of The Reward Range"

This concerns the entire liquidity amount being used for reward calculation rather than just tick based liquidity or in-reward-range liquidity.

Architechtural considerations:

Accounting

Here are three suggested changes to the accounting architecture. This is to address some of the problems raised in the issue submissions.

1. Only eligible liquidity positions should be included for the globalLiquidity used for rewards distribution.

2. The global liquidity calculation should be adjusted to ensure that the global eligible liquidity is greater or equal to the sum of all liquidity positions. Otherwise the rewards allocated would exceed the weekly allocated rewards, leading to reward insolvency.

3. Rewards for individual positions should be adjusted for tick size. pos.liquidity should be divided by upperTick - lowerTick

Gas

There is a larger overall design issue which is the decision to do on-chain liquidity mining by adding calls to reward accounting functions to every swap, mint and burn for a liquidity pool. This is extremely gas intensive and could affect the normal functioning of the protocol. It may even cause important functions to revert.

One simple solution can be done by off-chain rewarding. Perpetual Protocol, which is a concentrated liquidity Perpetual AMM built on-top of Uniswap v3, used off-chain calculations for their liquidity mining program: [LINK].(https://perpprotocol.mirror.xyz/0C9AYnB93z5AZ4c-phO8Wg7jXRFYLt3e9TiZ2VH3DLw) This has huge centralisation downsides, which may go against CATNO's ethos but is the simplest gas solution. Obviously, this is not a smart contract solution so may automatically be unacceptable.

The gas problems may have to be addressed through a complete architectural redesign rather than just normal gas optimisation. Here are some suggestions:

One immediate step would to pop() tickTrackingData as soon as the exitTimestamp == entryTimestamp. This happens to the last element of the array when crossTicks is called. Tracking this is unnecessary, because the tick was active for 0 blocks, and therefore the time delta and hence allocated rewards is zero.

The documentation stated that CANTO rewards are meant to be distributed for stable pools for this codebase. The term "stable" could have different interpretations, but this reccomendation assumes that this refers to stablecoin-like or pegged asset pairs such as stETH/WETH, USDT/USDC etc.

Instead of iterating through every tick, one could assume a range where the stable assets could lie and then reward all positions that lie within the specified range - in this case +- 10 ticks of the price tick.

This makes an assumption that these "stable assets" will actually stay pegged to each other. However, the current accounting architecture has multiple problems:

• Given the high number of loops required by the current accounting mechanism, there are multiple reasons that gas could run out. This includes iterating through too many ticks or having too many tick entries/exits

• The current mechanism increases the gas costs of all minting, burning and harvesting

• DOS attacks like the one described in this issue are possible.

Assuming a stable price has the downside of misallocating rewards if the stable assets depeg from each other.

Time spent:

25 hours