`rangeSecondsInside` underflow

[code423n4]

Handle

broccoli

Vulnerability details

rangeSecondsInside undeflow

Impact

The function rangeSecondsInside ConcentratedLiquidityPool.sol#L635-L658 would revert the transaction in some cases.

The ticks' secondsPerLiquidityOutside is only set when the pool cross it. (Ticks.sol#L23-L53)[https://github.com/sushiswap/trident/blob/c405f3402a1ed336244053f8186742d2da5975e9/contracts/libraries/concentratedPool/Ticks.sol#L23-L53]

When the pool never crosses a tick, the function might break.

rangeSecondsInside is designed to handle staking rewards. This function should never revert the transaction in any case. Though this may not harm the pool itself, a revert transaction may break other protocols. (Or ConcentratedLiquidityPoolManager). I consider this is a high-risk issue.

Proof of Concept

    global_ticks_list = [tick_min]
    add_liquidity(pool, deposit_amount, -400, 500)
    add_liquidity(pool, deposit_amount, 400, 700)
    add_liquidity(pool, deposit_amount, 600, 700)

    # swapping to set tick 500's time
    swap(pool, False, 2 * deposit_amount)
    swap(pool, True, 2 * deposit_amount)

    add_liquidity(pool, deposit_amount, 200, 500)

    # revert
    pool.functions.rangeSecondsInside(200, 500).call()

Tools Used

Hardhat

Recommended Mitigation Steps

The fix is whether to set ticks secondsPerLiquidityOutside or to handle this in rangeSecondsInside.

A possible quick fix may be like:

if (secondsGlobal < secondsBelow + secondsAbove) {
    return 0;
}

[sarangparikh22]

The example is wrong, you can't add use upper tick as odd, correct the example and resubmit please.

[alcueca]

@sarangparikh22, is the example invalid, or the whole issue? Is this something that you would consider fixing?

[sarangparikh22]

@alcueca As per my testing, the above case will not hold as the secondsGlobal is always greater in any case. The example depicted is also incorrect.

[alcueca]

Agree with sponsor