Kyber Network KyberSwap Conditional Swap Hook Security Review Report

When dstFeeRate == 0, the hook measures the change in the recipient's token-out balance to determine whether the swap satisfies minPrice. However, this includes any tokens received during the action, not just the swap output.

According to the documented trust model, the delegated key is untrusted and can choose the swap route and fee rates within the limits committed to the Merkle leaf. A malicious delegated key can therefore execute a swap that pays less than the user's minimum price and use an external payout to make the hook accept it.

Root cause

In beforeExecution, the hook snapshots the balance of the recipient when dstFeeRate == 0.

holderBalanceBefore: tokenOut.balanceOf(
    _settlementHolder(swapHookData.recipient, intentDstFeeRate)
),
 
function _settlementHolder(address recipient, uint256 feeRate) internal view returns (address) {
    return feeRate != 0 ? msg.sender : recipient;
}

In afterExecution, it calculates the output from the difference between the recipient's current balance and that snapshot:

uint256 amountOut = tokenOut.balanceOf(
    _settlementHolder(validationData.recipient, validationData.dstFeeRate)
) - validationData.holderBalanceBefore;
 
uint256 netAmountOut = amountOut - dstFee;
uint256 netExecutionPrice = Math.mulDiv(
    netAmountOut, OracleLib.PRICE_SCALE, amountIn
);

The hook then checks netExecutionPrice against minPrice. Because the measurement covers the entire action, tokens received from other sources can satisfy the price check.

The documented invariant that the realized price is based on the actual swap output is therefore not enforced in this settlement mode.

Example

Alice signs an intent to sell 1 ETH for at least $2,970. Her employer also pays her salary through a Sablier stream, from which approximately $2,900 is available to withdraw.

The delegated key chooses a route that transfers Alice's ETH to the attacker, pays Alice $100 in USDC, and calls Sablier.withdrawMax(streamId, alice) to withdraw her accrued salary.

The hook observes approximately $3,000 in additional USDC and accepts the swap, even though the swap itself paid only $100. Alice loses her 1 ETH for $100, and the withdrawn salary makes the execution appear to satisfy her minimum price.

Always measure swap output at the router, regardless of dstFeeRate. Have the hook return the output tokens, fees and amounts to the router so it can transfer the net output to the committed recipient.

This ensures that only tokens received through the swap settlement count toward minPrice, preventing unrelated transfers to the recipient from satisfying the price check.

The afterExecution hooks validate zap-outs from Uniswap V3 and V4 positions. After a whitelisted executor handles the position, each hook checks that the NFT was returned if liquidity remains and that the recipient received enough output for the liquidity removed.

A valid zap-out should only count proceeds from removed liquidity toward the minimum rate. Any fees already earned by the position belong to the user and should be returned separately, even when no liquidity is removed.

However, the hooks do not require any liquidity to be removed and do not account for accrued fees. When liquidityBefore == liquidityAfter, the minimum-output check becomes outputAmount >= 0 and always passes:

liquidity = liquidityBefore - liquidityAfter;
if (outputAmount * RATE_DENOMINATOR < minRate * liquidity) revert BelowMinRate(...);

Collecting a V3 position, or decreasing a V4 position by zero, can still realize all accrued fees. A whitelisted executor can therefore collect those fees to an address chosen in the zap call, return the unchanged NFT, and pass every hook check. The same missing fee accounting also lets accrued fees cover part of the required output during a partial exit.

An exploit can happen as follows:

1. A user's V3 or V4 position accrues fees and becomes eligible for execution.
2. A whitelisted executor submits a zap-out that removes zero liquidity and sends the collected fees to its chosen recipient.
3. The position NFT is returned to the user with all liquidity intact.
4. Since the removed liquidity and required output are both zero, afterExecution accepts the execution while the executor keeps the fees.

Require liquidityBefore - liquidityAfter to be greater than zero. Also calculate the position's accrued fees on-chain and require the recipient to receive those fees in addition to the minimum output for the removed liquidity.

KSConditionalSwapHook.getSwapExecutionCount() uses leafIndex to select an execution counter.

function getSwapExecutionCount(bytes32 intentHash, uint256 leafIndex)
    public
    view
    returns (uint256)
{
    uint256 packedValue = swapRecord[intentHash][leafIndex / 32];
    uint256 bytePosition = leafIndex % 32;
    ...
}

The name leafIndex suggests a position in the Merkle tree, but it represents a logical counter identifier committed within the leaf hash. Its numerical value does not determine tree position: a leaf with identifier 0 may appear to the right of a leaf with identifier 1.

An integration that interprets this identifier as a tree position may submit the position instead of the committed identifier. When these values differ, beforeExecution() computes a different leaf hash and reverts with InvalidProof, preventing the affected execution.

Rename leafIndex to conditionId consistently across leaf hashing, action decoding, callback data, and counter access.

Represent committed identifiers and tree positions separately in leaf construction and action encoding code.

These functions prepare output validation for Uniswap V4 and PancakeSwap V4 CL liquidity removals. They read the two pool currencies, normalize native-token addresses, and record the router's balances before the position manager removes liquidity. After removal, BaseTickBasedRemoveLiquidityHook._validateOutput calculates each output from the balance increase, and HookLibrary.afterExecution transfers both outputs to the position owner.

Normally, the two output entries represent two different assets. Each balance increase should therefore be counted once and only the assets received from the removal should be paid to the user.

However, a pool can use address(0) as its native currency and TokenHelper.NATIVE_ADDRESS as its other currency. V4 considers these different addresses, but the hooks normalize both to TokenHelper.NATIVE_ADDRESS. If wrapping is enabled, both are then normalized to WETH instead. The hook does not check that the resulting output tokens are still different.

As a result, _recordRouterBalances reads the same asset balance twice. A single increase of X is reported as two increases of X, and HookLibrary.afterExecution transfers both amounts. The second transfer comes from native tokens or WETH already held by the router.

An attacker can exploit this as follows:

1. Create a V4 pool using address(0) and TokenHelper.NATIVE_ADDRESS, then mint a one-sided position funded only with native currency.
2. Submit a valid removal intent while the router already holds native tokens or WETH, depending on the wrapping setting.
3. Remove X from the position. Both output slots record the same X balance increase.
4. The settlement loop sends X twice, returning the attacker's principal and taking another X from the router's existing balance. Only pre-existing balance in the router is affected.

function _recordRouterBalances(address router, address[2] memory tokens)
    internal
    view
    returns (uint256 balance0, uint256 balance1)
{
    balance0 = tokens[0].balanceOf(router);
    balance1 = tokens[1].balanceOf(router);
}

After all native-token and WETH normalization, require the two output token addresses to be different in both V4 hooks. As defense in depth, settlement should also reject or combine duplicate output-token entries before transferring funds.

The Uniswap V3 zap hook checks the pool price before execution and measures how much liquidity was removed afterward. These values are used to keep execution within the user's signed price range and minimum output rate.

The configured offsets are ABI return-word indexes. The readers should skip the 32-byte bytes length field and then move by 32 bytes for each word. For example, offset 7 should read word 7 from positions(), while offset 0 should read the first word from slot0().

However, both readers add the configured index directly to the bytes pointer:

liquidity := mload(add(result, liquidityOffset))
sqrtPriceX96 := mload(add(result, priceOffset))

This treats a word index as a raw byte offset and does not skip the length field. With the offsets used by the repository test, slot0() returns the constant return-data length 224 instead of the pool price. The liquidity reads also return the same header-derived value before and after execution, making the removed liquidity equal to zero. The minimum-rate check then always passes because it compares the output against minRate * 0.

An example flow is:

1. A user signs a zap intent with liquidity offset 7, price offset 0, and a broad price range that includes 224.
2. The hook accepts the fake price and records a header-derived liquidity value before execution.
3. The zap removes the position's liquidity and executes with an action-level output minimum below the intent's minimum rate, for example after a stale quote or sandwich.
4. The hook reads the same fake liquidity value afterward, calculates zero removed liquidity, and accepts the lower output. The separate action-level minimum still limits the final loss. A narrow, realistic pool-price range instead causes the zap to revert every time.

Use typed Uniswap V3 interface calls and destructure the required values. If configurable word indexes are needed, validate the return-data length and load each value from result + 0x20 + wordIndex * 0x20.