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

0 stars 1 forks source link

Limit `accrueConcentratedPositionTimeWeightedLiquidity` calls to prevent reward manipulation. #273

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

Vulnerability details

Impact

It may be possible for a user to artificially increase their tracked liquidity right before claiming by rapidly entering/exiting positions. This could allow them to claim a larger % of rewards than they deserve.

Proof of Concept

The main risk of manipulating own liquidity accounting occurs in these functions:

accrueConcentratedPositionTimeWeightedLiquidity function

function accrueConcentratedPositionTimeWeightedLiquidity(  
  //...
) internal {

  // Accrue user's liquidity accounting

}

This function is called whenever a position is modified to accrue the time-weighted liquidity for reward calculations.

The issue is there is no check on the frequency of calls. An attacker could exploit this by:

  1. Deposit small amount of liquidity
  2. Rapidly exit and re-enter position with much higher liquidity
  3. Call accrueConcentratedPositionTimeWeightedLiquidity
  4. Claim rewards based on higher liquidityaccounting

function accrueConcentratedPositionTimeWeightedLiquidity

function claimConcentratedRewards

function accrueConcentratedPositionTimeWeightedLiquidity(
  address owner,
  bytes32 poolIdx, 
  int24 lowerTick,
  int24 upperTick  
) internal {

  // Loops through tick history and accumulates time * liquidity
  // for the position's liquidity at each tick

}

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

  // Calls accrueConcentratedPositionTimeWeightedLiquidity
  // Calculates and claims rewards

}

The issue arises because accrueConcentratedPositionTimeWeightedLiquidity can be called with no limitations on frequency.

This allows an attacker to exploit the accounting process as follows:

  1. Attacker deposits a small amount of liquidity (ex: 10 ETH)

  2. Over a 1 week period, attract little accounting rewards proportional to 10 ETH

  3. Right before claiming rewards, rapidly exit position and re-deposit with 1000 ETH

  4. Call accrueConcentratedPositionTimeWeightedLiquidity

  5. This will register the 1000 ETH liquidity for the full week, instead of just the end

  6. Claim rewards based on owning 1000 ETH for the full week

To prevent this manipulation, accrueConcentratedPositionTimeWeightedLiquidity should limit calls to once per 24 hours.

Tools Used

Vs

Recommended Mitigation Steps

A limit needs to be added on how often accrueConcentratedPositionTimeWeightedLiquidity can be called per position. For example, limiting it to once per day would make this attack infeasible.

Assessed type

Timing

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