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

0 stars 1 forks source link

Protect against griefing by allowing only owner to manipulate global liquidity. #274

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

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

Vulnerability details

Impact

There don't seem to be protections against a malicious actor griefing others by manipulating the global liquidity accounting. This could potentially block honest users from claiming their earned rewards.

Proof of Concept

The main risk of griefing by manipulating global liquidity accounting stems from this function:

function accrueConcentratedGlobalTimeWeightedLiquidity

function accrueConcentratedGlobalTimeWeightedLiquidity(
  bytes32 poolIdx,
  CurveMath.CurveState memory curve  
) internal {

  // Accumulates global time-weighted liquidity

} 

The issue is this function can be called by any user at any time. There is no access control.

An attacker could exploit this by:

  1. Calling accrueConcentratedGlobalTimeWeightedLiquidity with manipulated curve liquidity data right before honest users try to claim rewards

  2. This distorts the global accounting that reward calculations rely on

  3. Honest users end up claiming smaller % of rewards due to griefing

A more in-depth explanation for the griefing attack would be very helpful here.


function accrueConcentratedGlobalTimeWeightedLiquidity(
   bytes32 poolIdx,
   CurveMath.CurveState memory curve
) internal {

  // Accumulates global time-weighted liquidity
  // Based on curve.concLiq_

}

function claimConcentratedRewards(
  address owner, 
  bytes32 poolIdx,
  // ...
) internal {

  accrueConcentratedGlobalTimeWeightedLiquidity(poolIdx, curve) 

  // Calculate rewards based on global liquidity accounting

}

function claimConcentratedRewards

The griefing attack works as follows:

  1. Honest user has been accruing rewards for 1 week based on depositing 100 ETH in pool liquidity

  2. Attacker has small portion of pool liquidity

  3. When honest user tries to claim rewards:

    3a. Attacker calls accrueConcentratedGlobalTimeWeightedLiquidity with a manipulated curve.concLiq_ value of 10 ETH

    3b. This results in a very low global liquidity for the week right before claim

  4. Honest user's rewards claim calculates percentage based on the distorted global liquidity

  5. Honest user gets only a small portion of the intended rewards due to griefing

Tools Used

Vs

Recommended Mitigation Steps

To prevent this, accrueConcentratedGlobalTimeWeightedLiquidity should check:


require(msg.sender == contractOwner, "Only owner can call");

This would ensure only the owner can accumulate global liquidity, preventing manipulation.

Assessed type

Access Control

c4-pre-sort commented 1 year ago

141345 marked the issue as low quality report

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Insufficient quality