Lines of code
<https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L62&gt;
<https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L43-L45&gt;
<https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L62-L65&gt;
<https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L43-L45&gt;
<https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L62-L64&gt;

Vulnerability details

Impact

timeWeightedWeeklyGlobalConcLiquidityLastSet_ is read and written in multiple functions. If two transactions call at similar times, the state updates could overwrite each other.

The timeWeightedWeeklyGlobalConcLiquidityLastSet_ state variable is used in these functions:

• accrueConcentratedGlobalTimeWeightedLiquidity *accrueConcentratedPositionTimeWeightedLiquidity

It is read here:

• Line 43-44:

      uint32 lastAccrued = timeWeightedWeeklyGlobalConcLiquidityLastSet_[
          poolIdx
      ];
  

And written here:

• Line 62-63:

      timeWeightedWeeklyGlobalConcLiquidityLastSet_[poolIdx] = uint32(
          block.timestamp
      );
  

These functions can be called independently by separate transactions.

If two transactions call accrueConcentratedGlobalTimeWeightedLiquidity concurrently, the lastAccrued value read by the second call may be outdated, leading to incorrect liquidity accruals.

Similarly, if accrueConcentratedPositionTimeWeightedLiquidity relies on an outdated lastAccrued, the liquidity sums can be wrong.

Proof of Concept

The timeWeightedWeeklyGlobalConcLiquidityLastSet_ state variable tracks the last time that the global concentrated liquidity was accrued.

It is used in two functions: Line 43-44,Line 62-63

// accrueConcentratedGlobalTimeWeightedLiquidity


  uint32 lastAccrued = timeWeightedWeeklyGlobalConcLiquidityLastSet_[poolIdx];

  // Accrue global liquidity since lastAccrued
  
  timeWeightedWeeklyGlobalConcLiquidityLastSet_[poolIdx] = block.timestamp;

}

// accrueConcentratedPositionTimeWeightedLiquidity 


  uint32 lastAccrued = timeWeightedWeeklyGlobalConcLiquidityLastSet_[poolIdx];  

  // Accrue position liquidity since lastAccrued

}

When these two functions are called concurrently by separate transactions:

• Tx1 calls accrueGlobal(), reads lastAccrued as 100, accrues liquidity, and sets lastAccrued to 200
• Tx2 calls accruePosition() concurrently, also reads lastAccrued as 100 (hasn’t seen Tx1 update yet)
• Tx2 performs incorrect accruals since its starting point is outdated
• Tx2 finishes and sets lastAccrued to 300, overwriting Tx1’s update

Now the global liquidity state updated by Tx1 is lost.

This race condition can lead to incorrect reward calculations.

Example Scenario:

contract Rewards {

  uint public lastAccrued;

  function accrueGlobal() external {
    uint last = lastAccrued;
    // Accrue rewards 
    lastAccrued = block.timestamp; 
  }

  function accrueUser() external {
    uint last = lastAccrued;
    // Accrue user rewards
    lastAccrued = block.timestamp;
  }

}

Now if we call accrueGlobal in Tx1 and accrueUser in Tx2 concurrently:

Tx1:

1. Calls accrueGlobal

2. Reads lastAccrued as 100

3. Accrues global rewards from 100 to current block 200

4. Sets lastAccrued to 200

Tx2:

1. Calls accrueUser

2. Reads lastAccrued as 100 (not aware of Tx1 update yet)

3. Accrues user rewards from 100 to current block 300

4. Sets lastAccrued to 300, overwriting Tx1’s update of 200

Now the global rewards accrued by Tx1 are lost. The lastAccrued update gets overwritten due to the race condition.

Tools Used

Manual

Recommended Mitigation Steps

Should consider using mutexes/locks or optimistic concurrency patterns like commit-reveal to prevent race conditions.

Assessed type

Reentrancy

---

The text was updated successfully, but these errors were encountered:

All reactions