Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Oracle Miscalculation During Zero Liquidity Periods Leads to Inaccurate AMM Data #41

Open
c4-bot-5 opened this issue Oct 11, 2024 · 0 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_primary AI based primary recommendation 🤖_44_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality

Comments

@c4-bot-5
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-10-ramses-exchange/blob/main/contracts/CL/core/libraries/Oracle.sol#L24

Vulnerability details

Description

The transform function is a core component of the Oracle system. It's used to update observations of the pool state over time, which are critical for accurate price feeds, liquidity calculations, and potentially reward distributions.

The issue, however, lies in the handling of the liquidity parameter when it's zero. The current implementation uses a ternary operator to avoid division by zero, but this approach can lead to inaccurate accumulation of secondsPerLiquidityCumulativeX128 when the pool has no liquidity.

Code

function transform(
    Observation memory last,
    uint32 blockTimestamp,
    int24 tick,
    uint128 liquidity
) private pure returns (Observation memory) {
    unchecked {
        uint32 delta = blockTimestamp - last.blockTimestamp;
        return Observation({
            blockTimestamp: blockTimestamp,
            tickCumulative: last.tickCumulative + int56(tick) * int56(uint56(delta)),
            secondsPerLiquidityCumulativeX128: last.secondsPerLiquidityCumulativeX128 +
                ((uint160(delta) << 128) / (liquidity > 0 ? liquidity : 1)),
            initialized: true
        });
    }
}

The issue is in this line:

((uint160(delta) << 128) / (liquidity > 0 ? liquidity : 1))

When liquidity is zero, we're dividing by 1 instead of zero to avoid a division-by-zero error. While this prevents the function from reverting, it introduces a logical error in the calculations.

Impact

  • Inaccurate Liquidity Tracking: During periods of zero liquidity, the function is still accumulating time as if there was 1 unit of liquidity. This leads to an overestimation of the time-weighted average liquidity.

  • Misleading Oracle Data: Any system relying on this oracle for liquidity data will receive inaccurate information. This could affect trading decisions, risk assessments, or any other mechanisms that depend on accurate liquidity reporting.

Scenario

Imagine a pool that alternates between having liquidity and being empty:

  • T=0: Liquidity = 1000
  • T=100: Liquidity drops to 0
  • T=200: Liquidity increases to 2000
  • T=300: Observation is taken

With our current implementation:

  • From T=0 to T=100, we correctly accumulate 100 seconds worth of liquidity.
  • From T=100 to T=200, we incorrectly accumulate 100 seconds as if there was 1 unit of liquidity.
  • From T=200 to T=300, we correctly accumulate 100 seconds worth of liquidity.

This results in an overestimation of the average liquidity over this period.

Fix

Modify the function to skip accumulation when liquidity is zero.

function transform(
    Observation memory last,
    uint32 blockTimestamp,
    int24 tick,
    uint128 liquidity
) private pure returns (Observation memory) {
    unchecked {
        uint32 delta = blockTimestamp - last.blockTimestamp;
        uint160 secondsPerLiquidityCumulativeX128 = last.secondsPerLiquidityCumulativeX128;
        if (liquidity > 0) {
            secondsPerLiquidityCumulativeX128 += ((uint160(delta) << 128) / liquidity);
        }
        return Observation({
            blockTimestamp: blockTimestamp,
            tickCumulative: last.tickCumulative + int56(tick) * int56(uint56(delta)),
            secondsPerLiquidityCumulativeX128: secondsPerLiquidityCumulativeX128,
            initialized: true
        });
    }
}

This change ensures that we only accumulate secondsPerLiquidityCumulativeX128 when there's actual liquidity in the pool.

Assessed type

Oracle

@c4-bot-5 c4-bot-5 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Oct 11, 2024
c4-bot-1 added a commit that referenced this issue Oct 11, 2024
@c4-bot-12 c4-bot-12 added the 🤖_primary AI based primary recommendation label Oct 11, 2024
howlbot-integration bot added a commit that referenced this issue Oct 13, 2024
@howlbot-integration howlbot-integration bot added the sufficient quality report This report is of sufficient quality label Oct 13, 2024
@c4-bot-12 c4-bot-12 added 🤖_44_group AI based duplicate group recommendation and removed 🤖_14_group labels Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_primary AI based primary recommendation 🤖_44_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

2 participants