Canto Liquidity Mining Protocol - kutugu's results

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L181 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

When calculating reward distribution, the contract uses the current position's liquidity time weight divided by the total liquidity time weight of the pool, instead of the total liquidity time weight of the pool that meets the reward conditions. This means that if there is a large amount of liquidity in the pool that is not eligible for rewards, the total weekly reward distribution will be much less than the rewardPerWeek, which should not be expected.

Proof of Concept

diff --git a/canto_ambient/test_canto/TestLiquidityMining.js b/canto_ambient/test_canto/TestLiquidityMining.js
index bd21a32..617b070 100644
--- a/canto_ambient/test_canto/TestLiquidityMining.js
+++ b/canto_ambient/test_canto/TestLiquidityMining.js
@@ -32,7 +32,7 @@ chai.use(solidity);
 
 describe("Liquidity Mining Tests", function () {
 	it("deploy contracts and init pool", async function () {
-		const [owner] = await ethers.getSigners();
+		const [owner, others] = await ethers.getSigners();
 
 		////////////////////////////////////////////////
 		// DEPLOY AND MINT cNOTE and USDC
@@ -49,6 +49,14 @@ describe("Liquidity Mining Tests", function () {
 			owner.address,
 			ethers.utils.parseUnits("1000000", 6)
 		);
+		await cNOTE.deposit(
+			others.address,
+			ethers.utils.parseEther("1000000")
+		);
+		await USDC.deposit(
+			others.address,
+			ethers.utils.parseUnits("1000000", 6)
+		);
 
 		////////////////////////////////////////////////
 		// DEPLOY DEX CONTRACT AND ALL PROXIES
@@ -145,6 +153,17 @@ describe("Liquidity Mining Tests", function () {
 		);
 		await approveCNOTE.wait();
 
+		approveUSDC = await USDC.connect(others).approve(
+			dex.address,
+			BigNumber.from(10).pow(36)
+		);
+		await approveUSDC.wait();
+		approveCNOTE = await cNOTE.connect(others).approve(
+			dex.address,
+			BigNumber.from(10).pow(36)
+		);
+		await approveCNOTE.wait();
+
 		/* 
         /	2. set new pool liquidity (amount to lock up for new pool)
         /	   params = [code, liq]
@@ -222,6 +241,40 @@ describe("Liquidity Mining Tests", function () {
 		});
 		await tx.wait();
 
+		mintConcentratedLiqCmd = abi.encode(
+			[
+				"uint8",
+				"address",
+				"address",
+				"uint256",
+				"int24",
+				"int24",
+				"uint128",
+				"uint128",
+				"uint128",
+				"uint8",
+				"address",
+			],
+			[
+				11, // code (mint concentrated liquidity in base token liq)
+				cNOTE.address, // base token
+				USDC.address, // quote token
+				36000, // poolIDX
+				currentTick - 5, // tickLower
+				currentTick + 5, // tickUpper
+				BigNumber.from("100000000000000000000"), // amount of base token to send
+				BigNumber.from("16602069666338596454400000"), // min price
+				BigNumber.from("20291418481080506777600000"), // max price
+				0, // reserve flag
+				ZERO_ADDR, // lp conduit address (0 if not using)
+			]
+		);
+		tx = await dex.connect(others).userCmd(2, mintConcentratedLiqCmd, {
+			gasLimit: 6000000,
+			value: ethers.utils.parseUnits("10", "ether"),
+		});
+		await tx.wait();
+
 		////////////////////////////////////////////////
 		// SAMPLE SWAP TEST (swaps 2 USDC for cNOTE)
 		////////////////////////////////////////////////
@@ -300,9 +353,9 @@ describe("Liquidity Mining Tests", function () {
 		const dexBalAfter = await ethers.provider.getBalance(dex.address);
 		const ownerBalAfter = await ethers.provider.getBalance(owner.address);
 
-		// expect dex to have 2 less CANTO since we claimed for 2 weeks worth of rewards
+		// @audit Expect to have less CANTO rewards due to additional and not eligible for rewards liquidity
 		expect(dexBalBefore.sub(dexBalAfter)).to.equal(
-			BigNumber.from("2000000000000000000")
+			BigNumber.from("501412401683494542")
 		);
 	});
 });

In the pool, there is only one position that meets the reward conditions, and there is also one position that does not meet the conditions. According to common sense, weekly rewards should be distributed to the only position that meets the reward conditions based on rewardPerWeek, but in reality, you can see that the rewards that should be distributed every week have been reduced to 1 / 4 due to the ineligible position.

Tools Used

Hardhat

Recommended Mitigation Steps

Since the reward distribution of concentrat type rewards is conditionally restricted, the conditional restrictions should also be taken into account when calculating the total weight, rather than directly counting the total amount of liquidity.

Assessed type

Context

Findings Summary

Detailed Findings

[L-01] The accrueTimeWeightedLiquidity may fail due to run out of gas

Description

This is very likely to happen, mainly for the accumulateConcentratedPositionTimeWeightedLiquidity function. The time range can be specified when the user claims the reward, but accumulateConcentratedPositionTimeWeightedLiquidity will update state from lastAccrued to the current time, and is not affected by the parameters passed in by the user. For a large tick range position that has not been updated for a long time, claiming the reward will consume a lot of gas and may cause the transaction to fail.

Recommendations

accumulateConcentratedPositionTimeWeightedLiquidity should only update the state for the specified time range

[L-02] A small amount unclaimed reward may be locked forever in contract

Description

LiquidityMiningPath allows the owner to transfer in ETH and set rewards, but it does not provide any method to withdraw unclaimed rewards in the contract, and the upper CrocSwapDex proxy contract does not have any support. Since the rewards are distributed according to the liquidity ratio, if some users never claim the rewards, the remaining rewards will be locked in the contract permanently, causing a loss of funds. Of course, rewards can continue to be distributed in the next cycle, but some of the rewards will always remain in the contract until the value is reduced to almost 0. If the owner does not want to distribute rewards any more, the remaining funds cannot be withdrawn.

Recommendations

Add a new method to withdraw funds

[N-01] Better to make sure crossTicks don't revert

Description

    /// @notice Keeps track of the tick crossings
    /// @dev Needs to be called whenever a tick is crossed
    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);
    }

crossTicks will be called every time the tick changes. If exitTickTracking is empty, crossTicks revert will DOS swap operation.
Although exitTickTracking should be generated by calling initTickTracking during initialization, it's better to verify that exitTickTracking is not empty in crossTicks.

Recommendations

Check numElementsExit > 0

[N-02] The user can't accrue rewards when week + WEEK == block.timestamp

Description

    for (uint256 i; i < weeksToClaim.length; ++i) {
        uint32 week = weeksToClaim[i];
        // @audit Revert when week + WEEK == block.timestamp
        require(week + WEEK < block.timestamp, "Week not over yet");
        require(
            !concLiquidityRewardsClaimed_[poolIdx][posKey][week],
            "Already claimed"
        );
    }

Recommendations

Use week + WEEK <= block.timestamp instead of week + WEEK < block.timestamp