Rebalancing on slot0.tick centres the band one tick off the price
| Title | Strategy main ticks are set according to the tick in slot0, leading to incorrect allocation and loss of funds |
|---|---|
| Reward | $3738, Unique |
| Contest | Yieldoor - 24 February 2025 on Sherlock |
| Author | 0x73696d616f (0xSimao) |
| Context | Tick precision |
Yieldoor is a concentrated liquidity manager. On every rebalance it picks a band around the current price and mints liquidity into it. Centring is the whole product: a band centred on the price holds roughly half its value in each token and earns fees on both sides of the market. Get the centre wrong and the position is lopsided from the moment it is minted.
So the number the strategy centres on has to be the price. rebalance() reads both numbers out of one slot0() call and then sends each to a different place:
204_removeLiquidity();205(uint256 amount0, uint256 amount1) = idleBalances();206207@>(uint160 sqrtPriceX96, int24 tick,,,,,) = IUniswapV3Pool(pool).slot0();208209@>_setMainTicks(tick);210@>(amount0, amount1) = _addLiquidityToMainPosition(sqrtPriceX96, amount0, amount1);211212_setSecondaryPositionsTicks(tick);213_addLiquidityToSecondaryPosition(sqrtPriceX96, amount0, amount1);214lastRebalance = block.timestamp;_setMainTicks picks the band, and it is a pure function of tick: it snaps the tick to the nearest boundary and hangs positionWidth / 2 off each side. _addLiquidityToMainPosition then fills that band using sqrtPriceX96.
Both values come from the same slot0() read. The band is chosen from tick and filled at sqrtPriceX96, and those two agree everywhere except on a tick boundary.
Two numbers, and only one of them is a price
Both come out of the same storage slot, and the pool labels them itself in Slot0:
56struct Slot0 {57 // the current price58@> uint160 sqrtPriceX96;59 // the current tick60@> int24 tick;61 // the most-recently updated index of the observations array62 uint16 observationIndex;63 // the current maximum number of observations that are being stored64 uint16 observationCardinality;65 // the next maximum number of observations to store, triggered in observations.write66 uint16 observationCardinalityNext;67 // the current protocol fee as a percentage of the swap fee taken on withdrawal68 // represented as an integer denominator (1/x)%69 uint8 feeProtocol;70 // whether the pool is locked71 bool unlocked;72}initialize writes both, deriving the tick from the price:
271function initialize(uint160 sqrtPriceX96) external override {272 require(slot0.sqrtPriceX96 == 0, 'AI');273274@> int24 tick = TickMath.getTickAtSqrtRatio(sqrtPriceX96);275276 (uint16 cardinality, uint16 cardinalityNext) = observations.initialize(_blockTimestamp());277278 slot0 = Slot0({279 sqrtPriceX96: sqrtPriceX96,280 tick: tick,281 observationIndex: 0,282 observationCardinality: cardinality,283 observationCardinalityNext: cardinalityNext,284 feeProtocol: 0,285 unlocked: true286 });287288 emit Initialize(sqrtPriceX96, tick);289}They do not stay that way, because a swap does not always write both. At the end of swap:
743 (slot0.sqrtPriceX96, slot0.tick, slot0.observationIndex, slot0.observationCardinality) = (744 state.sqrtPriceX96,745 state.tick,746 observationIndex,747 observationCardinality748 );749} else {750 // otherwise just update the price751@> slot0.sqrtPriceX96 = state.sqrtPriceX96;752}tick is only rewritten when it changed. Every swap that moves the price without leaving the current tick updates sqrtPriceX96 alone. tick therefore carries the price to a resolution of one tick and no better, which is the whole of what Uniswap promises about it:
Writing ratio for getSqrtRatioAtTick. Read it as a range, because that is what it is. Every tick is a cell with a boundary at each end, and the price lives somewhere inside the cell. The tick tells you which cell. It does not tell you where in the cell the price sits, and every price in the shaded stretch below returns the same tick.
Yieldoor mints a band four ticks wide, so one tick of error moves the centre by a quarter of the range. And there is one state where even the guarantee above stops holding.
Where the relationship breaks
There is one state where the invariant above does not hold at all, and a caller can put the pool into it deliberately.
The whole of it lives in the tail of the swap loop in UniswapV3Pool.swap, which runs once per price step. The two lines that write state.tick are marked:
692// shift tick if we reached the next price693if (state.sqrtPriceX96 == step.sqrtPriceNextX96) {694 // if the tick is initialized, run the tick transition695 if (step.initialized) {696 // check for the placeholder value, which we replace with the actual value the first time the swap697 // crosses an initialized tick698 if (!cache.computedLatestObservation) {699 (cache.tickCumulative, cache.secondsPerLiquidityCumulativeX128) = observations.observeSingle(700 cache.blockTimestamp,701 0,702 slot0Start.tick,703 slot0Start.observationIndex,704 cache.liquidityStart,705 slot0Start.observationCardinality706 );707 cache.computedLatestObservation = true;708 }709 int128 liquidityNet =710 ticks.cross(711 step.tickNext,712 (zeroForOne ? state.feeGrowthGlobalX128 : feeGrowthGlobal0X128),713 (zeroForOne ? feeGrowthGlobal1X128 : state.feeGrowthGlobalX128),714 cache.secondsPerLiquidityCumulativeX128,715 cache.tickCumulative,716 cache.blockTimestamp717 );718 // if we're moving leftward, we interpret liquidityNet as the opposite sign719 // safe because liquidityNet cannot be type(int128).min720 if (zeroForOne) liquidityNet = -liquidityNet;721722 state.liquidity = LiquidityMath.addDelta(state.liquidity, liquidityNet);723 }724725@> state.tick = zeroForOne ? step.tickNext - 1 : step.tickNext;726} else if (state.sqrtPriceX96 != step.sqrtPriceStartX96) {727 // recompute unless we're on a lower tick boundary (i.e. already transitioned ticks), and haven't moved728@> state.tick = TickMath.getTickAtSqrtRatio(state.sqrtPriceX96);729}Three branches, and only the first one is lossy.
The outer if fires when the price landed exactly on step.sqrtPriceNextX96, which is the boundary of step.tickNext. Line 725 then writes the tick: going up it is tickNext, going down zeroForOne is true and it is tickNext - 1. The price is sitting on the boundary in both cases, and only the downward branch steps back off it. That one wrote -1770 while sqrtPriceX96 was the ratio at -1769.
The else if is the ordinary case, where the price stopped somewhere inside a tick. There Uniswap does not guess. It derives the tick from the price with TickMath.getTickAtSqrtRatio, which is exactly what this finding ends up recommending the strategy do.
Getting the pool into that window costs one swap, because every router call takes a sqrtPriceLimitX96 and the loop respects it:
640// continue swapping as long as we haven't used the entire input/output and haven't reached the price limit641@>while (state.amountSpecifiedRemaining != 0 && state.sqrtPriceX96 != sqrtPriceLimitX96) {642 StepComputations memory step;643644 step.sqrtPriceStartX96 = state.sqrtPriceX96;645646 (step.tickNext, step.initialized) = tickBitmap.nextInitializedTickWithinOneWord(647 state.tick,648 tickSpacing,649 zeroForOne650 );651652 // ensure that we do not overshoot the min/max tick, as the tick bitmap is not aware of these bounds653 if (step.tickNext < TickMath.MIN_TICK) {654 step.tickNext = TickMath.MIN_TICK;655 } else if (step.tickNext > TickMath.MAX_TICK) {656 step.tickNext = TickMath.MAX_TICK;657 }658659 // get the price for the next tick660 step.sqrtPriceNextX96 = TickMath.getSqrtRatioAtTick(step.tickNext);661662 // compute values to swap to the target tick, price limit, or point where input/output amount is exhausted663 (state.sqrtPriceX96, step.amountIn, step.amountOut, step.feeAmount) = SwapMath.computeSwapStep(664 state.sqrtPriceX96,665@> (zeroForOne ? step.sqrtPriceNextX96 < sqrtPriceLimitX96 : step.sqrtPriceNextX96 > sqrtPriceLimitX96)666 ? sqrtPriceLimitX96667 : step.sqrtPriceNextX96,668 state.liquidity,669 state.amountSpecifiedRemaining,670 fee671 );Line 641 stops the loop the moment state.sqrtPriceX96 reaches the limit. Line 665 is what makes that exact: each step swaps towards the closer of the next initialized tick and the limit, so a limit set to getSqrtRatioAtTick(tickNext) is both at once. The step ends on the boundary, the loop condition then holds, and the swap returns with the price sitting exactly there.
An attacker picks that limit deliberately. Nothing about it is unusual: it is the same parameter every aggregator sets for slippage.
Impact
The band is supposed to be symmetric around the price. This one is minted three ticks below and one above, which has two consequences and they compound.
The position holds the wrong ratio of the two tokens the moment it is created, so the rebalance that was meant to reset it to roughly 50/50 does the opposite. And the position sits one tick from going out of range on the upper side, where any trade at all pushes it out. Out of range, concentrated liquidity earns nothing. The strategy then keeps that dead position until the next rebalance.
The attacker's side of it is cheap. Landing the pool on a boundary is one swap with a chosen sqrtPriceLimitX96, and the rebalancer does the rest.
Alpha: when a protocol reads a value another protocol derives, go and read what the deriving protocol actually guarantees about it, rather than what the name implies. slot0.tick is not a rounded copy of slot0.sqrtPriceX96. It is a separate field the pool writes only when the tick changes, and on a downward cross it writes tickNext - 1 rather than the tick the price is actually in. The one state where the two disagree is a state any caller can force for the price of a single swap, which is exactly where somebody will go looking. The same question is worth asking of every slot0 field, every latestRoundData field and every convertToAssets result you have ever taken at face value.
The proof
Fork Base at block 26874136. The pool starts at tick -1769. One swap, limited to exactly that boundary:
function test_POC_WrongTicks_DueToNotUsingSqrtPriceX96() public {
(uint160 sqrtPriceX96, int24 tick,,,,,) = pool.slot0();
assertEq(tick, -1769);
//@audit swap to clear current tick token0 liquidity
uint256 amountToSwap = 100e18;
deal(address(wbtc), depositor, amountToSwap);
vm.startPrank(depositor);
IMainnetRouter.ExactInputSingleParamsV2 memory swapParams;
swapParams.tokenIn = address(wbtc);
swapParams.tokenOut = address(usdc);
swapParams.recipient = depositor;
swapParams.fee = 100;
swapParams.amountIn = amountToSwap;
swapParams.sqrtPriceLimitX96 = TickMath.getSqrtRatioAtTick(-1769);
wbtc.approve(uniRouter, amountToSwap);
IMainnetRouter(uniRouter).exactInputSingle(swapParams);
vm.startPrank(rebalancer);
skip(10 minutes);
IStrategy(strategy).rebalance();
IStrategy.Position memory mainPos = IStrategy(strategy).getMainPosition();
(sqrtPriceX96, tick,,,,,) = pool.slot0();
assertEq(sqrtPriceX96, TickMath.getSqrtRatioAtTick(tick + 1)); //@audit price is in tick -1769 actually
assertEq(tick, -1770); //@audit but current tick is 1 more
assertEq(mainPos.tickLower, -1772);
assertEq(mainPos.tickUpper, -1768);Three assertions carry the finding. sqrtPriceX96 equals the ratio at tick + 1, so the price is at the -1769 boundary. slot0.tick reads -1770. And the rebalance, reading the stale number, minted the band at -1772 to -1768.
The rest of the test swaps one wei back the other way, and the pool's tick moves to -1769. One wei is the entire distance between the freshly minted position and the edge of its own range.
Conclusion
The fix is one line: derive the tick from the price with TickMath.getTickAtSqrtRatio(sqrtPriceX96) and centre on that. It paid $3738 as a unique high, and it came from reading a Uniswap invariant carefully enough to notice the one boundary where it does not hold.
If ticks, Q64.96 prices and the swap loop are not yet second nature, the Uniswap V3 Development Book is the clearest walkthrough there is. It rebuilds v3 from scratch, and the chapters on tick maths and on the swap loop are what make a bug like this one visible rather than surprising.