Kernel auth gap on Withdraw — PoC adapted for current master#6
Closed
saroupille wants to merge 1 commit into
Closed
Kernel auth gap on Withdraw — PoC adapted for current master#6saroupille wants to merge 1 commit into
saroupille wants to merge 1 commit into
Conversation
…alance on master The earlier PoC on analysis/withdraw-auth-gap-poc used the deposit shortcut `encode_ticket_deposit_message(victim, amount)` to populate a victim's public balance in a single step. Since upstream commit 8983481 ("Bind shield deposits to secret-derived keys") the kernel rejects any deposit whose recipient key is not `deposit:<32-byte-hex>`, so that shortcut no longer reaches the Withdraw handler. `KernelWithdrawReq` is still `{sender, recipient, amount}` on master (origin/main @ 1bea26b) — no signature, no proof. The auth gap the original PoC documented is still live; the secret-bound-deposit check is a partial patch for the deposit sub-case only. This adapted PoC seeds the kernel store directly (bridge ticketer + victim's balance at `/tzel/v1/state/balances/by-key/hex(tz1…)`) to reach the state a legitimate unshield would have produced, then submits an unauthenticated Withdraw from a third party. The drain succeeds, the victim's balance is zeroed / deleted, and an outbox message addressed to the attacker is enqueued. Why seeded state instead of fixture-driven shield+unshield: the `verified_bridge_flow.json` fixture is out of sync with master (all fixture-based tests in `bridge_flow.rs` currently panic under the `proof-verifier` feature). Seeding bypasses the fixture drift and isolates the bug of interest — once balance exists, anyone drains. Remove this test once the kernel gains sender authentication on Withdraw (design/tezos-auth-shield-withdraw). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to saroupille#1. That PR documented a reproducible
auth gap on
KernelInboxMessage::Withdraw; the kernel-side PoC itshipped used the shortcut
encode_ticket_deposit_message(victim, amount)to populate thevictim's public balance in one step, and that shortcut no longer
reaches the Withdraw handler because upstream commit
89834818("Bind shield deposits to secret-derived keys") rejects deposits
whose recipient key is not
deposit:<32-byte-hex>.This PR ships a new test that reaches the same state via direct
store seeding (bridge ticketer + victim's balance written to
/tzel/v1/state/balances/by-key/<hex(addr)>) and shows the authgap is still present on current master.
What the test does
tezos/rollup-kernel/tests/bridge_flow.rs :: withdraw_poc_v2_drains_seeded_public_balance_without_authPATH_BRIDGE_TICKETERso the Withdrawhandler can encode its outbox message.
500_001to the victim's balance key — the same state alegitimate unshield would have produced.
KernelWithdrawReq { sender: victim_tz1, recipient: attacker_tz1, amount: 500_001 }.KernelResult::Withdraw(_), victim's balance is now 0 orabsent, and an outbox withdrawal record addressed to the attacker
was enqueued for
500_001mutez.Run:
No feature flag, no fixture proofs. The
proof-verifier-gatedfixture tests on master currently panic at the first config step
(
verified_bridge_flow.jsonappears out of sync with the code); theseeded approach avoids that drift.
Evidence on master (unchanged since PR #1)
core/src/kernel_wire.rs:110-115—KernelWithdrawReq { sender, recipient, amount }. No signature, no proof.tezos/rollup-kernel/src/lib.rs:1004-1028— Withdraw match arm:checks
!is_deposit_balance_key(sender), reads ticketer, checksbalance >= amount, writes outbox, debits. Nothing bindssenderto the caller.
The
is_deposit_balance_keycheck added by89834818narrows theattack surface (a
deposit:<hex>key cannot be drained via Withdraw)but does not address the general case: once any unshield has credited
a tz1 balance, that balance is drainable by anyone who can submit an
inbox message — any Tezos L1 account holder — because the rollup
kernel has no access to the L1 source of the inbox message and the
request itself carries no auth.
Impact
any party able to submit an inbox message to the rollup, e.g. via
octez-client send smart rollup messagefrom any funded L1 account.deposit:<hex>balances are safe from direct drain(per the commit 8983481 guard) but still transit through a
drainable state after unshield.
require_bearer_auth(services/tzel/src/bin/tzel_operator.rs:304) is bypassed because the attack submits via
octez-clientdirectly, not through the operator.Fix space
Not proposing a fix in this PR. Relevant direction, already sketched
on
design/tezos-auth-shield-withdraw: bind each Withdraw to a proofthe caller owns
sender. Two families discussed there:KernelWithdrawReqwith asignature field; kernel verifies the sig against
sender(whensenderis a tz1/KT1). Requires a canonical Withdraw-messageserialization to sign.
already uses for private notes, lifted to public balances. Larger
change, PQ-compatible.
Either would remove the need for the
is_deposit_balance_keyguard — deposit and public-balance keys could converge into one
namespace once Withdraw is authenticated.
What this PR does NOT propose
analysis/withdraw-auth-gap-poc; thissits alongside it as the current-master variant.
Verification artefacts
tezos/rollup-kernel/tests/bridge_flow.rs(+108 lines, one new test, no other changes).
1bea26b.🤖 Generated with Claude Code