Canto Liquidity Mining Protocol - maanas's results

Lines of code

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

Vulnerability details

Impact

If a user provides liquidity on ticks which are entered and exited a large number of times, the gas required to call the accrueConcentratedPositionTimeWeightedLiquidity can exceed the block gas limit.

Proof of Concept

The accrueConcentratedPositionTimeWeightedLiquidity function loops over the unbounded TickTracking array inside tickTracking_ mapping.

                // Loop through all in-range time spans for the tick or up to the current time (if it is still in range)
                while (time < block.timestamp && tickTrackingIndex < numTickTracking) {
                    TickTracking memory tickTracking = tickTracking_[poolIdx][i][tickTrackingIndex];

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

The TickTracking array is pushed with a new value everytime the corresponding tick is entered.

Hence a possible combination of too many tick movements and user's not collecting rewards/updating position for a long time can result in the amount of gas required to iterate the loop exceeding the block gas limit.

Demo:

With 4000 cross tick movements across two ticks before the user claim rewards, the gas used to call the claimConcentratedRewards function is 34M which is above the block gas limit of 30M.

To execute the demo the following changes has to be made to the current repo:

1. Update ProtocolCmd.sol: Done to add a new command which will allow to cross the tick

diff --git a/canto_ambient/contracts/libraries/ProtocolCmd.sol b/canto_ambient/contracts/libraries/ProtocolCmd.sol
index 21a408d..7641c69 100644
--- a/canto_ambient/contracts/libraries/ProtocolCmd.sol
+++ b/canto_ambient/contracts/libraries/ProtocolCmd.sol
@@ -119,4 +119,5 @@ library UserCmd {
     ////////////////////////////////////////////////////////////////////////////
     uint8 constant CLAIM_CONC_REWARDS_CODE = 101;
     uint8 constant CLAIM_AMB_REWARDS_CODE = 102;
+    uint8 constant CROSS_TICKS = 103;
 }

2. Update LiquidityMiningPath.sol: Done to add the command to cross the ticks

diff --git a/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol b/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol
index e6c63f7..bb511c9 100644
--- a/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol
+++ b/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol
@@ -46,11 +46,15 @@ contract LiquidityMiningPath is LiquidityMining {
             claimConcentratedRewards(poolHash, lowerTick, upperTick, weeksToClaim);
         } else if (code == UserCmd.CLAIM_AMB_REWARDS_CODE) {
             claimAmbientRewards(poolHash, weeksToClaim);
+        } else if (code == UserCmd.CROSS_TICKS){
+            crossTicks(poolHash,lowerTick,upperTick);
         } else {
             revert("Invalid user command");
         }
     }
 
+    
+
     function claimConcentratedRewards(bytes32 poolIdx, int24 lowerTick, int24 upperTick, uint32[] memory weeksToClaim)
         public
         payable

3. Update TestLiquidityMining.js: Commented out the swap and added loop to continously cross the tick. Test passes if the gas used is more than 30M

diff --git a/canto_ambient/test_canto/TestLiquidityMining.js b/canto_ambient/test_canto/TestLiquidityMining.js
index bd21a32..0e9c0eb 100644
--- a/canto_ambient/test_canto/TestLiquidityMining.js
+++ b/canto_ambient/test_canto/TestLiquidityMining.js
@@ -31,6 +31,8 @@ function toSqrtPrice(price) {
 chai.use(solidity);
 
 describe("Liquidity Mining Tests", function () {
+
+	this.timeout(3000000);
 	it("deploy contracts and init pool", async function () {
 		const [owner] = await ethers.getSigners();
 
@@ -225,23 +227,23 @@ describe("Liquidity Mining Tests", function () {
 		////////////////////////////////////////////////
 		// SAMPLE SWAP TEST (swaps 2 USDC for cNOTE)
 		////////////////////////////////////////////////
-		swapTx = await dex.swap(
-			cNOTE.address, // base
-			USDC.address, // quote
-			36000, // poolIdx
-			false, // isBuy
-			false, // inBaseQty
-			BigNumber.from("2000000"), // qty
-			0, // tip
-			BigNumber.from("16602069666338596454400000"), // limit price
-			BigNumber.from("1900000000000000000"), // min out
-			0 // reserveFlag (to use surplus or not)
-		);
-
-		await swapTx.wait();
-		expect(await USDC.balanceOf(owner.address)).to.equal(
-			BigNumber.from("999898351768")
-		);
+		// swapTx = await dex.swap(
+		// 	cNOTE.address, // base
+		// 	USDC.address, // quote
+		// 	36000, // poolIdx
+		// 	false, // isBuy
+		// 	false, // inBaseQty
+		// 	BigNumber.from("2000000"), // qty
+		// 	0, // tip
+		// 	BigNumber.from("16602069666338596454400000"), // limit price
+		// 	BigNumber.from("1900000000000000000"), // min out
+		// 	0 // reserveFlag (to use surplus or not)
+		// );
+
+		// await swapTx.wait();
+		// expect(await USDC.balanceOf(owner.address)).to.equal(
+		// 	BigNumber.from("999898351768")
+		// );
 
 		//////////////////////////////////////////////////
 		// SET LIQUIDITY MINING REWARDS FOR CONCENTRATED LIQUIDITY
@@ -270,7 +272,46 @@ describe("Liquidity Mining Tests", function () {
 		tx = await dex.protocolCmd(8, setRewards, true);
 		await tx.wait();
 
-		await time.increase(604800 * 5); // fast forward 1000 seconds so that rewards accrue
+        // make the ticks exit and reenter
+        for(let i=1;i<=4000;i++){           
+            
+            let tickMover = abi.encode(
+                    ["uint8", "bytes32", "int24", "int24", "uint32[]"],
+                    [
+                        103,
+                        keccak256(poolHash),
+                        currentTick ,
+                        currentTick +1,
+                        [
+                            Math.floor(timestampBefore / 604800) * 604800 + 604800,
+                            Math.floor(timestampBefore / 604800) * 604800 + 604800 * 2,
+                        ],
+                    ]
+                );
+                tx = await dex.userCmd(8, tickMover);
+                await tx.wait();
+    
+                await time.increase(100);
+                tickMover = abi.encode(
+                    ["uint8", "bytes32", "int24", "int24", "uint32[]"],
+                    [
+                        103,
+                        keccak256(poolHash),
+                        currentTick +1,
+                        currentTick ,
+                        [
+                            Math.floor(timestampBefore / 604800) * 604800 + 604800,
+                            Math.floor(timestampBefore / 604800) * 604800 + 604800 * 2,
+                        ],
+                    ]
+                );
+             
+            tx = await dex.userCmd(8, tickMover);
+            await tx.wait();
+    
+            await time.increase(100);
+        }
+        await time.increase(604800 * 5);
 
 		//////////////////////////////////////////////////
 		// CLAIM REWARDS ACCRUED FROM CONCENTRATED REWARDS
@@ -294,7 +335,9 @@ describe("Liquidity Mining Tests", function () {
 			]
 		);
 		tx = await dex.userCmd(8, claim);
-		await tx.wait();
+		const receipt = await tx.wait();
+		console.log(receipt.gasUsed);
+		expect(receipt.gasUsed > 30000000).to.be.eq(true);
 
 		// get eth balanace of dex after claim
 		const dexBalAfter = await ethers.provider.getBalance(dex.address);

Tools Used

Hardhat

Recommendations

If possible find an alternative method to calculate the user's concentrated liquidity or warn the user's about this issue.

Assessed type

Loop

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol#L65 https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol#L74

Vulnerability details

Impact

The owner of liquidity mining sidecar can pull the native coins that are stored in the CrocSwapDex to reward the users.

Proof of Concept

The setConcRewards and setAmbRewards functions doesn't check if the quoted amount of rewards are actually sent by the caller. This allows the owner to specify any total amount of native coin which available in the CrocSwapDex from which the funds will be used when distributing the rewards.

    function setConcRewards(bytes32 poolIdx, uint32 weekFrom, uint32 weekTo, uint64 weeklyReward) public payable {
        // require(msg.sender == governance_, "Only callable by governance");
        require(weekFrom % WEEK == 0 && weekTo % WEEK == 0, "Invalid weeks");
        while (weekFrom <= weekTo) {
            concRewardPerWeek_[poolIdx][weekFrom] = weeklyReward;
            weekFrom += uint32(WEEK);
        }
    }

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol#L65C7-L72

    function setAmbRewards(bytes32 poolIdx, uint32 weekFrom, uint32 weekTo, uint64 weeklyReward) public payable {
        // require(msg.sender == governance_, "Only callable by governance");
        require(weekFrom % WEEK == 0 && weekTo % WEEK == 0, "Invalid weeks");
        while (weekFrom <= weekTo) {
            ambRewardPerWeek_[poolIdx][weekFrom] = weeklyReward;
            weekFrom += uint32(WEEK);
        }
    }

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol#L74-L81

According to Ambient Docs they allow for deposits in native tokens.

Demo

1. Update TestLiquidityMining.js : The funds added using hardhat.setBalance() is being used by the owner to distribute rewards

diff --git a/canto_ambient/test_canto/TestLiquidityMining.js b/canto_ambient/test_canto/TestLiquidityMining.js
index bd21a32..b917308 100644
--- a/canto_ambient/test_canto/TestLiquidityMining.js
+++ b/canto_ambient/test_canto/TestLiquidityMining.js
@@ -7,6 +7,7 @@ const { time } = require("@nomicfoundation/hardhat-network-helpers");
 var keccak256 = require("@ethersproject/keccak256").keccak256;
 
 const chai = require("chai");
+const { network, ethers } = require("hardhat");
 const abi = new AbiCoder();
 
 const BOOT_PROXY_IDX = 0;
@@ -218,7 +219,6 @@ describe("Liquidity Mining Tests", function () {
 		);
 		tx = await dex.userCmd(2, mintConcentratedLiqCmd, {
 			gasLimit: 6000000,
-			value: ethers.utils.parseUnits("10", "ether"),
 		});
 		await tx.wait();
 
@@ -243,6 +243,17 @@ describe("Liquidity Mining Tests", function () {
 			BigNumber.from("999898351768")
 		);
 
+		let dexBal = await ethers.provider.getBalance(dex.address);
+		expect(dexBal.eq(0)).to.be.eq(true);
+
+		// dex gains native token from other methods
+		await network.provider.send("hardhat_setBalance", [
+			dex.address,
+			ethers.utils.parseEther("2").toHexString(),
+		  ]);
+		dexBal = await ethers.provider.getBalance(dex.address);
+		expect(dexBal.eq(ethers.utils.parseEther("2"))).to.be.eq(true);
+
 		//////////////////////////////////////////////////
 		// SET LIQUIDITY MINING REWARDS FOR CONCENTRATED LIQUIDITY
 		//////////////////////////////////////////////////

Tools Used

Hardhat

Recommended Mitigation Steps

Add a msg.value check in the rewards function to see that the total value is passed when call the functions.

diff --git a/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol b/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol
index e6c63f7..44dd338 100644
--- a/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol
+++ b/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol
@@ -65,6 +65,7 @@ contract LiquidityMiningPath is LiquidityMining {
     function setConcRewards(bytes32 poolIdx, uint32 weekFrom, uint32 weekTo, uint64 weeklyReward) public payable {
         // require(msg.sender == governance_, "Only callable by governance");
         require(weekFrom % WEEK == 0 && weekTo % WEEK == 0, "Invalid weeks");
+        require((1 +(weekTo - weekFrom) / WEEK) * weeklyReward == msg.value);
         while (weekFrom <= weekTo) {
             concRewardPerWeek_[poolIdx][weekFrom] = weeklyReward;
             weekFrom += uint32(WEEK);

Assessed type

Rug-Pull