Kyber Network Smart Intent Protocol Security Review Report

The KSSmartIntentRouter contract validates execution signatures (dkSignature and gdSignature) using a hash derived solely from the ActionData structure. Crucially, this hash calculation does not incorporate the intentHash of the Intent being executed.

function _execute(
    bytes32 intentHash,
    IntentData calldata intentData,
    bytes calldata dkSignature,
    address guardian,
    bytes calldata gdSignature,
    ActionData calldata actionData
) internal nonReentrant {
    _checkIntentStatus(intentHash, IntentStatus.DELEGATED);
    if (actionData.actionSelectorId >= intentData.coreData.actionContracts.length) {
        revert InvalidActionSelectorId(actionData.actionSelectorId);
    }
    if (block.timestamp > actionData.deadline) {
        revert ActionExpired();
    }
    _checkRole(KSRoles.GUARDIAN_ROLE, guardian);
 
    _useUnorderedNonce(intentHash, actionData.nonce);
 
    _validateActionData(
        intentData.coreData,
        dkSignature,
        guardian,
        gdSignature,
        _hashTypedDataV4(hasher.hash(actionData))
    );

The router verifies DK/Guardian signatures over the ActionData hash only, without including the intentHash. As a result, a valid signature produced for one intent can be reused to execute a different intent that uses the same delegated key/guardian.

Per-intent status and nonce checks do not prevent this replay because they are scoped to the target intent, not the signature itself.

function _useUnorderedNonce(bytes32 intentHash, uint256 nonce) internal {
    uint256 wordPos = nonce >> 8;
    uint8 bitPos = uint8(nonce);
 
    uint256 bit = 1 << bitPos;
    uint256 flipped = nonces[intentHash][wordPos] ^= bit;
    if (flipped & bit == 0) {
        revert NonceAlreadyUsed(intentHash, nonce);
    }
 
    emit UseNonce(intentHash, nonce);
}

This breaks intent-specific authorization and can cause execution under different token sets, hooks, or action targets than the signers intended.

Bind action signatures to the intent by including intentHash in the signed message (or by signing a typed structure that incorporates it), and validate both DK and Guardian signatures against that combined hash across all supported verification paths.

The remove-liquidity hooks for Uniswap V3, V4 and Pancakeswap V4-CL use feesGenerated values decoded from actionData.hookActionData when evaluating yield-based conditions. These values are supplied by the delegated key and are not verified against on-chain state or any trusted source.

function _cacheBaseData(
    RemoveLiquidityHookData calldata validationData,
    bytes calldata hookActionData,
    RemoveLiquidityParams memory removeLiqParams,
    OutputValidationParams memory outputParams
) internal view virtual {
    (
        removeLiqParams.index,
        removeLiqParams.positionInfo.feesGenerated[0],
        removeLiqParams.positionInfo.feesGenerated[1],
        removeLiqParams.liquidityToRemove,
        removeLiqParams.wrapOrUnwrap,
        outputParams.intentFeesPercent
    ) = _decodeHookActionData(hookActionData);
    removeLiqParams.recipient = validationData.recipient;
    removeLiqParams.positionInfo.nftAddress = validationData.nftAddresses[removeLiqParams.index];
    removeLiqParams.positionInfo.nftId = validationData.nftIds[removeLiqParams.index];
 
    outputParams.router = msg.sender;
    outputParams.maxFees = [
        validationData.maxFees[removeLiqParams.index] >> 128,
        uint128(validationData.maxFees[removeLiqParams.index])
    ];
}

As a result, a delegated key can submit inflated fee values to satisfy the yield threshold even when actual fees are lower, allowing liquidity removal that does not match the main address's intended yield condition.

function _evaluateYieldCondition(Condition calldata condition, bytes calldata additionalData)
    internal
    pure
    virtual
    returns (bool)
{
    uint256 fee0;
    uint256 fee1;
    uint256 poolPrice;
 
    assembly ("memory-safe") {
        fee0 := calldataload(additionalData.offset)
        fee1 := calldataload(add(additionalData.offset, 0x20))
        poolPrice := calldataload(add(additionalData.offset, 0x40))
    }
 
    YieldCondition calldata yieldCondition = _decodeYieldCondition(condition.data);
 
    uint256 initialAmount0 = yieldCondition.initialAmounts >> 128;
    uint256 initialAmount1 = uint256(uint128(yieldCondition.initialAmounts));
 
    uint256 numerator = fee0 + _convertToken1ToToken0(poolPrice, fee1);
    uint256 denominator = initialAmount0 + _convertToken1ToToken0(poolPrice, initialAmount1);
    if (denominator == 0) return false;
 
    uint256 yield = (numerator * PRECISION) / denominator;
 
    return yield >= yieldCondition.targetYield;
}

Derive the fee inputs used for yield checks from on-chain state or other verifiable data instead of relying on hookActionData. If off-chain inputs are required, introduce validation that ties the submitted fee values to the position's observable state and reject mismatches.

Commentary from the client

We utilize the off-chain input fee to enhance the user experience. This fee accounts for both claimed and unclaimed amounts. Unclaimed fees alone can be reset to zero periodically, which is particularly relevant for Uniswap V4-like implementations.

When revoke is executed, the function updates intentStatuses to REVOKED, but it does not clean up the associated erc20Allowances. While this does not have a direct impact, it results in state inconsistency, where the intent is marked as REVOKED but the corresponding allowance data still exists.

/// @inheritdoc IKSSmartIntentRouter
function revoke(IntentData calldata intentData) public {
    if (intentData.coreData.mainAddress != msg.sender) {
        revert NotMainAddress();
    }
 
    bytes32 intentHash = _hashTypedDataV4(hasher.hash(intentData));
    intentStatuses[intentHash] = IntentStatus.REVOKED;
 
    emit RevokeIntent(intentHash);
}

Consider adding to clear allowance.

Commentary from the client

This behavior is intentional to reduce gas costs for the revoke action.

In KSSmartIntentRouterAccounting._approveTokens, ERC20 permit data is forwarded via callERC20Permit when provided, but the return value is not checked. The underlying helper (PermitHelper.callERC20Permit) uses try/catch and does not revert on failure, so a failed permit is silently ignored. As a result, execution can proceed without the intended allowance being set, and the transaction may revert later during token collection due to insufficient allowance.

function _approveTokens(bytes32 intentHash, TokenData calldata tokenData, address mainAddress)
    internal
{
    for (uint256 i = 0; i < tokenData.erc20Data.length; i++) {
        ERC20Data calldata erc20Data = tokenData.erc20Data[i];
 
        erc20Allowances[intentHash][erc20Data.token] = erc20Data.amount;
 
        if (erc20Data.permitData.length > 0) {
            erc20Data.token.callERC20Permit(mainAddress, erc20Data.permitData);
        }
    }
    for (uint256 i = 0; i < tokenData.erc721Data.length; i++) {
        ERC721Data calldata erc721Data = tokenData.erc721Data[i];
 
        if (erc721Data.permitData.length > 0) {
            erc721Data.token.callERC721Permit(erc721Data.tokenId, erc721Data.permitData);
        }
    }
}

Treat permit submission as a best-effort step rather than a hard requirement. When permit data is provided, attempt the permit call but gate execution on the post-call approval state (e.g., ensure the router/forwarder has sufficient allowance for the intended spend). If the required allowance is already sufficient, skip the permit call. If it is not sufficient after attempting permit, fail early with a clear error to surface the root cause.

ERC20DataLibrary.collect assumes that the receiver gets amount - fee after transferFrom. For fee-on-transfer (FOT) tokens, the actual received amount can be lower. This mismatch can cause the router or forwarder to have less balance than expected when the action contract runs, leading to failed executions or minor accounting inconsistencies (e.g., recorded volume/fees not matching actual transfers).

function collect(
    address token,
    uint256 amount,
    address mainAddress,
    address actionContract,
    uint256 fee,
    bool approvalFlag,
    IKSGenericForwarder forwarder,
    FeeConfig[] calldata partnerFeeConfigs,
    address protocolRecipient
) internal {
    if (address(forwarder) == address(0)) {
        token.safeTransferFrom(mainAddress, address(this), amount - fee);
        if (approvalFlag) {
            token.forceApprove(actionContract, type(uint256).max);
        }
    } else {
        token.safeTransferFrom(mainAddress, address(forwarder), amount - fee);
        if (approvalFlag) {
            _forwardApproveInf(forwarder, token, actionContract);
        }
    }
}

Update the collection flow to base subsequent logic on the actual amount received by the router/forwarder (e.g., using balance deltas), and align any accounting or validations to that effective amount.

The remove-liquidity hook decodes intentFeesPercent from hookActionData and applies it in fee calculations, while maxFees is only used to derive the minimum amount the user must receive. This means an explicit comparison between the two is missing. Although the current minReceived check indirectly enforces the limit and causes a revert when fees exceed maxFees, the absence of a direct check can make failures less clear.

function _cacheBaseData(
    RemoveLiquidityHookData calldata validationData,
    bytes calldata hookActionData,
    RemoveLiquidityParams memory removeLiqParams,
    OutputValidationParams memory outputParams
) internal view virtual {
    (
        removeLiqParams.index,
        removeLiqParams.positionInfo.feesGenerated[0]-
        removeLiqParams.positionInfo.feesGenerated[1],
        removeLiqParams.liquidityToRemove,
        removeLiqParams.wrapOrUnwrap,
        outputParams.intentFeesPercent
    ) = _decodeHookActionData(hookActionData);
    removeLiqParams.recipient = validationData.recipient;
    removeLiqParams.positionInfo.nftAddress = validationData.nftAddresses[removeLiqParams.index];
    removeLiqParams.positionInfo.nftId = validationData.nftIds[removeLiqParams.index];
 
    outputParams.router = msg.sender;
    outputParams.maxFees = [
        validationData.maxFees[removeLiqParams.index] >> 128,
        uint128(validationData.maxFees[removeLiqParams.index])
    ];
}

Add an explicit validation that the decoded intentFeesPercent values do not exceed the corresponding maxFees, and revert with a clear error when the limit is exceeded.