Skip to content

Wipe the sponge state after hashing so secrets don't linger on the stack - #85

Merged
illuzen merged 4 commits into
mainfrom
illuzen/zeroization
Aug 3, 2026
Merged

Wipe the sponge state after hashing so secrets don't linger on the stack#85
illuzen merged 4 commits into
mainfrom
illuzen/zeroization

Conversation

@illuzen

@illuzen illuzen commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Wipe the sponge state after hashing so secrets don't linger on the stack

Problem

Consumers of this crate hash secret material: a Quantus wormhole secret is
literally the output of hash_bytes, and the felt-encoded secret is the input
to the address-derivation hashes. A painted-stack probe in
qp-rusty-crystals (release-mode wormhole_stack_zeroization test) found
verbatim copies of the wormhole secret in dead stack memory after every
zeroizing wrapper on the caller's side had done its job. Bisection traced the
residue into this crate.

Root cause: the finalize pipeline was a chain of self-consuming methods —
finalize_to_bytes(self)finalize_to_felts(self)finalize_state(mut self). Each call moves the entire Poseidon2State (sponge state, buffered
input, permutation) by value into the callee's frame. Rust moves are copies:
every moved-from slot stays behind as a dead, never-dropped copy of the
sponge — which contains the squeezed digest and any buffered input block —
that no caller can ever reach or wipe. finalize_squeeze_twice additionally
built its output with [h1, h2].concat(), leaking the digest into a freed
heap block.

Fix

  • finalize_state / finalize_to_felts / finalize_to_bytes /
    finalize_squeeze_twice now take &mut self, so the sponge lives in
    exactly one stack slot for the whole hash computation. The byte finalizers
    pass digest_to_bytes a borrowed subarray of the state (try_into on
    the slice is a reference conversion, no copy) instead of an owned
    intermediate felt array, and the double squeeze writes into a fixed
    [u8; 64] instead of a heap Vec.
  • Every public hash function (hash_to_felts, hash_to_bytes, hash_bytes,
    hash_twice, rehash_to_bytes, hash_squeeze_twice) wipes the sponge —
    and any intermediate digest-felt buffer — before returning, so the only
    surviving copy of the digest is the returned value.
  • The wipe is safe code with no new runtime dependency:
    fill(Goldilocks::ZERO) routed through core::hint::black_box, which
    forces the compiler to materialize the zeroing stores instead of eliding
    them as dead. The regression test below pins that this survives codegen.

No behavior change

Hash outputs are bit-identical: the existing known-answer vectors
(test_known_value_hashes, test_hash_twice_vectors,
test_rehash_to_bytes_vectors, test_hash_squeeze_twice) pass unchanged.
The public API is unchanged; only private methods switched from self to
&mut self. The wipe cost (20 field-element stores per hash) is negligible
next to a Poseidon2 permutation.

Regression test

New tests/stack_zeroization.rs (release-mode only, psm dev-dependency):
runs a hash on a freshly painted stack buffer, then scans the buffer for the
known digest / input pattern.

  • hash_bytes_output_never_survives_on_stack — red before this change
    (2 residue copies), green after.
  • hash_twice_input_never_survives_on_stack — probes for sponge-internal
    copies of the input felts (the wormhole secret enters through this
    path) — red before (2 copies), green after.

Both tests include a self-check that the probe technique detects a
deliberately leaked copy.

Downstream

With qp-rusty-crystals patched to this branch, its
wormhole_stack_zeroization probe goes fully green (zero matches). Once this
ships (as =3.0.3), the downstream pin gets bumped and the temporary
[patch.crates-io] entry removed.


Note

High Risk
Touches secret-bearing hash paths (wormhole secrets); the fix is security-critical even though outputs stay bit-identical and the public API is unchanged.

Overview
Fixes dead stack copies of sponge state (digest + buffered secret input) left behind by the old self-consuming finalize chain. Finalization now runs in place via &mut self, byte digests are built from borrowed state slices (no extra felt array), and finalize_squeeze_twice writes into a stack [u8; 64] instead of a heap Vec.

Adds wipe / wipe_felts (fill + core::hint::black_box) and calls them from every public hash path (hash_to_felts, hash_to_bytes, hash_bytes, hash_twice, rehash_to_bytes, hash_squeeze_twice) so only the returned digest survives. Public API and hash outputs are unchanged (KATs unchanged).

New release-only tests/stack_zeroization.rs (dev-dep psm) painted-stack probes for hash_bytes output and hash_twice sponge-internal input residue.

Reviewed by Cursor Bugbot for commit 847033b. Configure here.

illuzen and others added 3 commits August 3, 2026 14:14
Consumers hash secret material (Quantus wormhole secrets are hash_bytes
outputs), but the self-consuming finalize chain moved the whole sponge -
buffered input and squeezed output - by value through three frames,
leaving dead unwipeable stack copies of the digest. Finalize now works
through &mut self, digests serialize straight from the state, and every
public hash function zeroes the sponge (fill + black_box, no unsafe)
before returning. Outputs are bit-identical (KATs unchanged); a
release-mode painted-stack probe pins the behavior.

Co-authored-by: Cursor <cursoragent@cursor.com>
…lper

digest_to_bytes already takes a reference, and try_into on the state
slice yields &[Goldilocks; 4] without copying, so the finalizers can use
the existing public serializer directly - no owned intermediate felt
array, and no duplicate encoding logic to keep in sync.

Co-authored-by: Cursor <cursoragent@cursor.com>

@n13 n13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Solid, well-scoped security fix. I verified it empirically, not just by reading:

  • Full release suite passes locally on macOS arm64 (KATs, serialization, non-canonical-limb tests, both new probes).
  • Re-ran the new probe tests against main's src/lib.rs: both fail as claimed (residue copies of the hash_bytes output and of the hash_twice input felts found in dead stack memory), and go green with this branch's lib.rs. The regression test is genuinely load-bearing, not vacuous.

What I checked

  • Mechanism. The self&mut self finalizer change removes the by-value moves of Poseidon2State through three frames. permute_mut already operates fully in place and bytes_to_felts_iter streams input without allocating, so post-fix the sponge really does live in exactly one wipeable slot. The comment's claim that self.state[..POSEIDON2_OUTPUT].try_into() is a reference conversion (&[Goldilocks]&[Goldilocks; 4]) is correct — no owned digest intermediate is created.
  • No behavior change. All KAT vectors pass unchanged; public API untouched; finalize_squeeze_twice reusing finalize_state also removes the previously duplicated padding loop.
  • Wipe coverage. wipe() covers state, buf, buf_len; the intermediate digest felts in hash_twice and rehash_to_bytes are wiped too; the [h1, h2].concat() heap leak is gone (fixed [u8; 64]).
  • Test/CI plumbing. psm is dev-only (zero downstream dependency footprint) and exact-pinned, consistent with the repo's supply-chain posture. CI runs cargo test --locked --release on both Linux and macOS, so the cfg(not(debug_assertions)) probe actually executes in CI. The probe's self-check (asserting the technique detects a deliberately leaked copy) is a nice touch. Wipe cost is negligible and constant-time (unconditional stores), so the dudect story is unaffected.

One design note (non-blocking)

core::hint::black_box is documented as best-effort only — the std docs explicitly say it must not be relied upon to control critical program behavior and offers no guarantees for security purposes. The semantically guaranteed alternative is volatile stores (ptr::write_volatile + compiler_fence, i.e. what zeroize does), at the cost of a small unsafe block or a new dependency.

I think shipping black_box here is defensible because the risk is fenced on both sides: the toolchain is pinned (1.93.0), this repo's probe pins the codegen on both CI targets, and the downstream qp-rusty-crystals probe covers the actual consumer build profile (including LTO/opt flags this repo's CI can't see). If a future toolchain bump lets dead-store elimination through, the probes go red instead of the wipe rotting silently. But that means the probes and the wipe strategy are coupled: if the probes are ever dropped or loosened, the wipe should move to volatile stores at the same time. Worth one line in the wipe_felts doc comment making that coupling explicit.

Minor

  • self.state[..POSEIDON2_OUTPUT].try_into().expect(...) now appears four times. A small borrowed accessor (e.g. fn digest(&self) -> &[Goldilocks; POSEIDON2_OUTPUT]) would deduplicate it without reintroducing an owned copy.
  • Poseidon2State has no Drop wipe, so a panic mid-hash would skip zeroization. Every operation on the hash path is infallible in practice, so this is theoretical — defense-in-depth to consider if the sponge ever grows fallible paths.
  • Known limitation, fine to leave as is: source-level zeroization can't reach spilled registers or sub-pattern fragments, and the probe detects only contiguous full-width copies. The doc phrase "no copy … survives this call's stack frame" holds at that granularity.

Verdict

Approve. Root cause correctly identified, minimal in-place fix, bit-identical outputs, and the regression test pins exactly the property the fix claims — verified red on the old code, green on this branch.

A borrowed digest() accessor replaces the four try_into repetitions, and
Poseidon2State now wipes in Drop, so the public hash functions no longer
need explicit wipe calls and a panic mid-hash can't skip zeroization.

Co-authored-by: Cursor <cursoragent@cursor.com>
@illuzen
illuzen merged commit 2128b46 into main Aug 3, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants