# Incorrect modulo calculation in secondary position ticks leads to active position and division by zero Medium · Sherlock · CLMM · 24th February 2025 Finding M-6 of the Yieldoor competition. - Protocol: https://audits.sherlock.xyz/contests/791 - Report: /reports/yieldoor - Codebase: https://github.com/0xsimao/2025-02-yieldoor/tree/b5a0f779dce4236b02665606adb610099451a51a - Source: https://github.com/sherlock-audit/2025-02-yieldoor-judging/issues/126 --- ## Summary The `Strategy#_setSecondaryPositionsTicks` function incorrectly handles negative ticks when calculating modulo, causing the secondary position to be active when it should be out-of-range and potentially leading to division by zero in `LiquidityAmounts#getLiquidityForAmounts`. ## Relevant Context The strategy maintains two Uniswap V3 positions: 1. A main position that should be balanced (50:50) 2. A secondary position that should always be out-of-range and filled with only one token The secondary position's ticks are set based on the current tick and token balances to ensure it remains out-of-range. The main position's ticks are calculated with proper handling of negative modulos, but this correction is missing in the secondary position calculation. [`_setMainTicks`](https://github.com/sherlock-audit/2025-02-yieldoor/blob/b5a0f779dce4236b02665606adb610099451a51a/yieldoor/src/Strategy.sol#L232) ```solidity /// @notice Calculates and sets the ticks of the main position /// @dev Checks if the nearest initializable tick is lower or higher than the current tick function _setMainTicks(int24 tick) internal { int24 halfWidth = int24(positionWidth / 2); int24 modulo = tick % tickSpacing; if (modulo < 0) modulo += tickSpacing; // if tick is negative, modulo is also negative bool isLowerSided = modulo < (tickSpacing / 2); int24 tickBorder = tick - modulo; if (!isLowerSided) tickBorder += tickSpacing; mainPosition.tickLower = tickBorder - halfWidth; mainPosition.tickUpper = tickBorder + halfWidth; emit NewMainTicks(tickBorder - halfWidth, tickBorder + halfWidth); } ``` [`_setSecondaryPositionsTicks`](https://github.com/sherlock-audit/2025-02-yieldoor/blob/b5a0f779dce4236b02665606adb610099451a51a/yieldoor/src/Strategy.sol#L366) ```solidity /// @notice Sets the secondary position ticks. /// @dev This position should always be created consisting of just one of the tokens /// @dev Should initially be Out-Of-Range function _setSecondaryPositionsTicks(int24 tick) internal { int24 modulo = tick % tickSpacing; uint256 bal0 = IERC20(token0).balanceOf(address(this)); uint256 bal1 = IERC20(token1).balanceOf(address(this)); uint256 _price = price(); uint256 bal0in1 = bal0 * _price / PRECISION; // usually either of them should be 0, but might be non-zero due to rounding when minting if (bal0in1 < bal1) { secondaryPosition.tickLower = mainPosition.tickLower; secondaryPosition.tickUpper = tick - modulo; } else { secondaryPosition.tickLower = tick - modulo + tickSpacing; secondaryPosition.tickUpper = mainPosition.tickUpper; // TODO check if these need to be reversed. } emit NewSecondaryTicks(secondaryPosition.tickLower, secondaryPosition.tickUpper); } ``` ## Root Cause In `Strategy#_setSecondaryPositionsTicks`, the modulo calculation for negative ticks is incorrect: [`Strategy.sol#L367`](https://github.com/sherlock-audit/2025-02-yieldoor/blob/b5a0f779dce4236b02665606adb610099451a51a/yieldoor/src/Strategy.sol#L367) ```solidity int24 modulo = tick % tickSpacing; ``` Unlike `Strategy#_setMainTicks` which handles negative modulos correctly: [`Strategy.sol#L235`](https://github.com/sherlock-audit/2025-02-yieldoor/blob/b5a0f779dce4236b02665606adb610099451a51a/yieldoor/src/Strategy.sol#L235) ```solidity if (modulo < 0) modulo += tickSpacing; ``` This leads to two issues: 1. When `bal0in1 < bal1`, the secondary position becomes active because `tick < tickUpper = tick - modulo` 2. When `bal0in1 >= bal1`, the secondary position range is shorter than intended because `tick - (tickSpacing + modulo) + tickSpacing < tick - modulo + tickSpacing` Additionally, when `positionWidth = 4 * tickSpacing` and `isLowerSided` is true, `tickLower` equals `tickUpper`, causing a division by zero in `LiquidityAmounts#getLiquidityForAmounts`. ## Attack Path 1. Initial conditions: - `positionWidth = 4 * tickSpacing` - Current tick is negative - `bal0in1 >= bal1` - Current tick results in `isLowerSided = true` in main position 2. When `rebalance()` is called: - `_setSecondaryPositionsTicks` calculates incorrect modulo - Sets `tickLower = tick - modulo + tickSpacing` - Sets `tickUpper = mainPosition.tickUpper` - Due to incorrect modulo, `tickLower = tickUpper` 3. `_addLiquidityToSecondaryPosition` attempts to add liquidity: - Calls `LiquidityAmounts#getLiquidityForAmounts` - Division by zero occurs due to equal ticks - Transaction reverts ## Impact Explanation High. The vulnerability has two severe impacts: 1. Secondary position becomes active when it should be out-of-range, breaking a core invariant of the strategy 2. Strategy becomes unusable when specific conditions are met due to division by zero ## Proof of Concept ### Case 1: bal0in1 < bal1 Consider: 1. `tickSpacing = 60` 2. Current tick = -121 3. `modulo = -121 % 60 = -1` (incorrect) 4. `bal0in1 < bal1` Calculations: - `secondaryPosition.tickLower = mainPosition.tickLower` - `secondaryPosition.tickUpper = -121 - (-1) = -120` - Current tick (-121) is less than tickUpper (-120) - Result: Secondary position becomes active when it should be out-of-range With correct modulo handling: - `modulo = -1 + 60 = 59` - `secondaryPosition.tickUpper = -121 - 59 = -180` - Current tick (-121) would be greater than tickUpper (-180) - Result: Secondary position remains out-of-range as intended ### Case 2: bal0in1 >= bal1 (with isLowerSided = true) Consider: 1. `tickSpacing = 60` 2. Current tick = -91 3. `positionWidth = 240` (4 * tickSpacing) 4. `modulo = -91 % 60 = -31` (incorrect) 5. `bal0in1 >= bal1` Main position calculations: - `modulo = -31 + 60 = 29` (corrected in _setMainTicks) - `isLowerSided = 29 < (60/2) = true` - `tickBorder = -91 - 29 = -120` - `mainPosition.tickUpper = -120 + 120 = 0` Secondary position calculations with incorrect modulo: - `tickLower = -91 - (-31) + 60 = 0` - `tickUpper = mainPosition.tickUpper = 0` - Result: `tickLower = tickUpper = 0`, causing division by zero With correct modulo handling: - `modulo = -31 + 60 = 29` - `tickLower = -91 - 29 + 60 = -60` - `tickUpper = mainPosition.tickUpper = 0` - Result: Valid position range (-60, 0) ## Recommendation Add the same modulo correction as in `_setMainTicks`: ```diff function _setSecondaryPositionsTicks(int24 tick) internal { int24 modulo = tick % tickSpacing; + if (modulo < 0) modulo += tickSpacing; uint256 bal0 = IERC20(token0).balanceOf(address(this)); uint256 bal1 = IERC20(token1).balanceOf(address(this)); uint256 _price = price(); uint256 bal0in1 = bal0 * _price / PRECISION; if (bal0in1 < bal1) { secondaryPosition.tickLower = mainPosition.tickLower; secondaryPosition.tickUpper = tick - modulo; } else { secondaryPosition.tickLower = tick - modulo + tickSpacing; secondaryPosition.tickUpper = mainPosition.tickUpper; } emit NewSecondaryTicks(secondaryPosition.tickLower, secondaryPosition.tickUpper); } ``` --- Related findings: - [Taker fee is underestimated due to incorrect fee calculation](https://0xsimao.com/findings/1inch-taker-fee-underestimated-calculation): 1inch Fee Extension - [Vault portion calculation in `PrizePool::getVaultPortion()` is incorrect as `_startDrawIdInclusive` has been erased](https://0xsimao.com/findings/pooltogether-the-prize-layer-for-defi-portion-prize-inclusive-erased): PoolTogether Prize Layer - [Incorrect withdraw queue balance in TVL calculation](https://0xsimao.com/findings/renzo-withdraw-queue-tvl-calculation): Renzo - [Incorrect initial deposit calculation may cause cancelProgram to revert](https://0xsimao.com/findings/superfluid-locker-system-ii-initial-deposit-program-revert): Superfluid Locker Pumponomics