[illuzen]

Summary

Fixes the state_call remote DoS from GHSA-4wm4-px2f-3cr5.

The shared gate in resolve_runtime_call_block (node/src/zktree_rpc.rs) only windowed ZkTreeApi_get_merkle_proof for untrusted callers and forwarded every other runtime API straight to the executor at the best block. state_call is a safe-classified RPC that never runs check_if_safe, so an unauthenticated public caller could run heavy, unmetered runtime APIs for free:

• GenesisBuilder_build_state — full serde parse (twice) plus per-account genesis storage writes.
• BlockBuilder_apply_extrinsic — reaches the wormhole pre_dispatch plonky2 verify that pool admission deliberately keeps off the fee-free path.

Both run in CallContext::Offchain with no weight metering, so a few-KB request can burn seconds of node CPU with no account and no fee.

The RPC-v2 chainHead_v1_call is a sibling of the same surface: client/rpc-spec-v2/src/chain_head/chain_head.rs passes the caller's function straight to executor().call(.., Offchain) with no check_if_safe, so it was reachable the same way (review finding).

Fix

Invert the untrusted-caller policy from default-allow to default-deny + allowlist, with one shared source of truth in sc_rpc_spec_v2::runtime_call_policy:

• PUBLIC_RUNTIME_CALL_ALLOWLIST / is_public_runtime_call_allowed — the cheap, read-only APIs public tooling (subxt, wallets, explorers) relies on: Core_version, Metadata_*, AccountNonceApi_account_nonce, TransactionPaymentApi_*, TransactionPaymentCallApi_*.
• is_trusted_caller(ext) — trust follows the per-connection DenyUnsafe extension; a missing extension is treated as untrusted (upstream's check_if_safe marks it unreachable!, so absence is a wiring error and fails closed).
• is_runtime_call_allowed(ext, method) — trusted ⇒ anything; untrusted ⇒ allowlist only.

Applied on every surface that lets a caller run a runtime API by name:

• state_call / state_callAt / archive_v1_call (node gate resolve_runtime_call_block): trusted callers keep any method at any block; untrusted callers get the windowed ZkTreeApi_get_merkle_proof, the allowlist at their chosen block, and error 9009 for everything else.
• chainHead_v1_call (in-tree handler): refuses non-allowlisted methods from untrusted callers with InvalidRuntimeCall (-32802) before the executor task is spawned, so no operation starts and no follow event is emitted. It pins recent blocks, so it doesn't need the deep-history window.

Why an allowlist rather than disabling the methods

Turning them off would also break the legitimate public reads that make them "safe" RPCs upstream. A denylist of known-bad methods would re-create the same default-allow trap one name at a time. Fail-closed is the point — new runtime APIs are denied to the public by default and have to be deliberately allowlisted.

Tests

• sc_rpc_spec_v2::runtime_call_policy (5 pass): allowlist membership; heavy/execution APIs denied; trust derivation for DenyUnsafe::No / Yes / missing; the combined decision for untrusted (Yes and missing) vs trusted callers over both method sets.
• node/src/zktree_rpc.rs gate tests (21 pass): the allow/deny/trusted matrix through resolve_runtime_call_block and end-to-end through the archive_v1_call handler, now on the shared policy.
• chain_head/tests.rs::call_runtime_denied_for_untrusted_callers: handler-level regression with DenyUnsafe::Yes — heavy methods refused synchronously, allowlisted read completes via the follow stream, trusted caller reaches the executor. Note: the vendored test-helpers suite does not currently build in this fork (its substrate-test-runtime-client dev-deps were stripped when the crate was vendored; test-helpers = [] is a stub), so this test is written against the existing harness/patterns but is not runnable here. The runnable coverage of the decision wired into the handler is the runtime_call_policy unit tests above.

./clippy.sh (fmt, taplo, workspace clippy) clean.

Severity note

Classified medium: availability-only, transient, and requires the operator to have exposed public RPC (default binds to localhost). The amplification is real (unauthenticated, unmetered, repeatable) but smaller than the report's headline figures; see the advisory discussion.

[n13]

Verdict: Request changes
Reviewer: GPT-6.1 Sol

Reviewed the complete 22-file diff at e016dc9d65feac44a44b6a2fdd3c77b6131bcf4a, including the inherited leaf-splitting and transaction-weight changes.

[P1] Apply the same runtime-API policy to chainHead_v1_call. The new gate in node/src/zktree_rpc.rs:247–265 protects the state and archive replacements, but the service still registers the original chain-head handler. client/rpc-spec-v2/src/chain_head/chain_head.rs:463–538 checks the connection's subscription, pinned block, and with_runtime flag, then passes the caller's arbitrary function directly to executor().call(..., CallContext::Offchain) at line 521. Neither the follow subscription nor this handler checks DenyUnsafe or the allowlist.

The source-traced reproduction is an unauthenticated WebSocket connection to public/safe RPC: subscribe to chainHead_v1_follow with [true], take a hash from the initialized subscription, then call chainHead_v1_call with [subscription_id, hash, "GenesisBuilder_build_state", "0x087b7d"] (0x087b7d is SCALE-encoded Vec<u8> containing {}). This path reaches the runtime instead of rejecting the method with 9009. The same entry point accepts the larger genesis inputs or BlockBuilder_apply_extrinsic payloads that motivated this fix, preserving fee-free, unmetered execution. The 16-operation subscription limit does not bound the CPU cost of an invocation or prevent repeated invocations. This is an unfixed sibling path for the same security issue.

Factor the public runtime-API policy into a gate that also runs before the chain-head executor call, preserving allowed reads and trusted access. Add a handler-level regression with DenyUnsafe::Yes that rejects the heavy methods before executor invocation, alongside allowed-read and trusted-call cases.

Validation: git diff --check, pinned scripts/fmt.sh --all -- --check, and locked offline Cargo metadata passed. A standalone Rust harness using the exact changed chunk/gate function bodies with minimal chain/error/log stubs passed 9 tests, including 100,000 randomized native-credit checks for conservation, cap, count, and quantization. These are focused logic checks, not node integration tests. timeout 60 env SKIP_WASM_BUILD=1 CARGO_TARGET_DIR=target/review cargo test --locked --offline -p pallet-zk-tree --lib reached the 60-second limit during dependency compilation without reaching tests. No live DoS was run. GitHub format/dependency checks passed; the build/test and analysis jobs were still running at review time.

[illuzen]

Addressed the P1 in 0666519.

• Factored the policy into sc_rpc_spec_v2::runtime_call_policy (allowlist, is_trusted_caller, is_runtime_call_allowed) so there is one definition for every runtime-call surface.
• chainHead_v1_call now runs is_runtime_call_allowed(ext, &function) before spawning the executor task; untrusted callers get InvalidRuntimeCall (-32802) for non-allowlisted methods, allowlisted reads and trusted callers are unchanged.
• The node's state_call / archive_v1_call gate consumes the same module instead of its local PUBLIC_STATE_CALL_ALLOWLIST, and its trust derivation now goes through the shared is_trusted_caller. Both gates fail closed when the DenyUnsafe extension is missing.
• Tests: runtime_call_policy unit tests cover the full DenyUnsafe::Yes / No / missing × heavy / allowlisted matrix; the existing 21 node gate tests pass on the shared policy; added call_runtime_denied_for_untrusted_callers to chain_head/tests.rs. Caveat on that last one: the vendored test-helpers suite doesn't build in this fork today (the substrate-test-runtime-client dev-deps were stripped on vendoring; test-helpers = [] is a stub, 264 pre-existing errors), so it's written against the existing harness but can't be executed here. Happy to drop it if you'd rather not carry an unrunnable test, or to take wiring the suite up as a follow-up.

[n13]

Verdict: Approve
Reviewer: GPT-6.1 Sol

Reviewed the complete 27-file PR diff at 0666519ec47e3d10b4e02c05e9f789059892f060, including the inherited leaf-splitting and transaction-weight changes. No remaining blocking findings.

The earlier P1 is addressed: chainHead_v1_call now checks the shared policy before starting the executor task. Rejection drops the block/operation guards, releasing the reservation. state_call, its state_callAt alias, and the archive replacement consume the same allowlist and trust derivation. Missing DenyUnsafe extensions fail closed; explicit trusted connections retain unrestricted runtime calls. The existing public proof-window checks remain in the state/archive gate.

Validation:

• git diff --check, pinned scripts/fmt.sh --all -- --check, taplo format --check, and locked offline Cargo metadata passed.
• An isolated Rust logic harness passed 13 tests: the exact policy module's five tests, six existing chunk tests, iterator-length checks, and 100,000 randomized native-credit checks for conservation, cap, count, and quantization. It used the real HTTP Extensions type and an extracted DenyUnsafe enum as dependency scaffolding; this does not exercise node/server integration.
• timeout 60 env SKIP_WASM_BUILD=1 CARGO_TARGET_DIR=target/review cargo test --locked --offline -p sc-rpc-spec-v2 --lib runtime_call_policy reached the 60-second limit during dependency compilation, before tests ran. The added chain-head handler regression is behind the vendored test-helpers feature and was not executed here.
• GitHub formatting, dependency-cooldown, and Clippy/docs checks passed. Both build/test jobs were still running at review time.

[illuzen]

Heads-up: the branch was accidentally created off wormhole-leaf-amount-cap-split rather than main, so the PR was carrying seven unrelated wormhole/runtime commits. Rebased onto current main (now f9b15d0a + 15b63708, same content as e016dc9d + 0666519e); the diff is now just the RPC policy change. ./clippy.sh and the gate tests re-run clean on top of main.

[n13]

Verdict: Approve
Reviewer: GPT-6.1 Sol

Reviewed the complete nine-file diff at 15b63708b03a9bd0d01d94400ec8071ea09001bb against base aae53e3693ab244c86cf3f10bb39e39500f5c6c2. No blocking findings.

The shared allowlist and DenyUnsafe trust derivation cover state_call, its state_callAt alias, the archive replacement, and chainHead_v1_call. Missing trust extensions fail closed. The chain-head check rejects non-allowlisted methods before spawning the executor; returning an error drops the block guard and releases its operation permit and pin. Trusted connections retain unrestricted runtime calls, allowed public reads retain their selected block, and the state/archive Merkle-proof window remains enforced. I also checked the service registration and server extension injection for bypasses.

Validation:

• git diff --check, pinned scripts/fmt.sh --all -- --check, taplo format --check --config taplo.toml, and locked offline Cargo metadata passed.
• All five tests in the unchanged runtime_call_policy.rs passed when compiled directly with rustc --test, using the cached real jsonrpsee dependency and the exact DenyUnsafe enum extracted from sc-rpc-api 0.54.0 as dependency scaffolding. This verifies the policy decisions, not server/handler integration.
• timeout 60 env SKIP_WASM_BUILD=1 CARGO_TARGET_DIR=target/review cargo test --locked --offline -p sc-rpc-spec-v2 --lib runtime_call_policy stopped at the 60-second limit during dependency compilation, before tests ran. The added chain-head handler regression is behind the existing test-helpers feature, whose SDK test-runtime dependencies are absent from the vendored manifest, and was not executed.
• GitHub dependency-cooldown checks passed; the formatting job was still running and build/test/analysis results were not yet available at review time.