code-423n4 / 2023-10-canto-findings

0 stars 1 forks source link

The Liquidity mining callpath sidecar owner can pull native tokens from the Dex #295

Open c4-submissions opened 11 months ago

c4-submissions commented 11 months ago

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") );


## 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
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

c4-pre-sort commented 11 months ago

141345 marked the issue as duplicate of #163

141345 commented 11 months ago

not quite the same as https://github.com/code-423n4/2023-10-canto-findings/issues/163

this one focus on rug pull, rather than reward balance

c4-pre-sort commented 11 months ago

141345 marked the issue as sufficient quality report

c4-judge commented 11 months ago

dmvt marked the issue as not a duplicate

c4-judge commented 11 months ago

dmvt marked the issue as primary issue

c4-judge commented 11 months ago

dmvt marked the issue as selected for report

c4-sponsor commented 10 months ago

OpenCoreCH (sponsor) acknowledged

OpenCoreCH commented 10 months ago

Rewards will be set and sent in the same Canto governance proposal