V12 again

[illuzen]

Address V12 audit findings (v12 report)

Fixes all six findings from the V12 audit. Each fix landed as its own commit with a regression test confirming the issue before the fix was applied.

Security fixes

Unbounded byte-hash allocation (#96388, Medium) — hash_bytes and hash_squeeze_twice materialized the entire serialized preimage (~3x input size across three heap buffers) before absorbing into the sponge, allowing memory-amplification DoS on attacker-sized inputs. Byte absorption now streams 4-byte chunks directly into the sponge via a new shared bytes_to_felts_iter / bytes_to_u64s_iter encoding iterator, keeping heap usage O(1) while producing byte-identical digests (verified against existing known-answer vectors).

Non-canonical 8-byte limb decoding (#96398, Medium) — bytes_to_digest and bytes_to_felts_compact accepted raw u64 limbs without checking value < P. Since P and 0 are the same Goldilocks element, byte-distinct inputs (e.g. a limb of P vs 0) decoded to identical felts and collided under rehash_to_bytes and compact hashing. These decoders now reject non-canonical limbs.

Panicking amount conversion (#96399, Medium) — u128_to_quantized_felt panicked via assert! on amounts whose quantized value exceeds 32 bits, turning input validation into a DoS vector (especially under panic=abort). Replaced by try_u128_to_quantized_felt, which returns a recoverable error, mirroring the existing reverse conversion.

Digest limb truncation (#96397, Low) — u64s_to_digest narrowed each u64 limb with as u32, silently discarding high bits so distinct limb arrays aliased to the same wire digest. It now rejects limbs exceeding 32 bits.

Benchmark discards timed outputs (#96386, Low) — every timed closure in the dudect constant-time harness (examples/ct_bench.rs) discarded its result, permitting the optimizer to elide the very work being measured. All inputs and outputs are now routed through black_box.

Constant-time documentation (#96400, Medium, audit PoC marked invalid) — the README claimed "no input-dependent branches," but Goldilocks add/sub/reduce contain rare carry/borrow correction branches (standard optimized Goldilocks arithmetic, as in plonky2/plonky3). The audit's own measurements found no exploitable timing distinguisher, so rather than a risky branchless rewrite, the claim is narrowed: dudect results are empirical evidence of no measurable leakage, not a branch-free guarantee.

Breaking API changes

Decoding/parsing functions at trust boundaries are now fallible, consistent with the existing try_* / Result conventions in the crate:

Function	Before	After
bytes_to_digest	infallible	Result — rejects limbs >= P
bytes_to_felts_compact	infallible	Result — rejects limbs >= P
rehash_to_bytes	infallible	Result — propagates digest decode errors
u128_to_quantized_felt	panics when oversized	renamed try_u128_to_quantized_felt, returns Result
u64s_to_digest	truncates silently	Result — rejects limbs > u32::MAX

Digests produced by this library are always canonical, so well-behaved callers can ?/expect these results. A semver-major release is warranted.

Tests

Four new integration test files (alloc_bounds, non_canonical_limbs, quantized_amount, digest_limb_truncation) covering each finding, including a tracking-allocator harness asserting O(1) heap for byte hashing. Full suite: 72 tests passing, clippy clean, dudect harness verified running in release mode.

---

Note

High Risk
Breaking public APIs plus cryptographic parsing and hash-input handling changes; callers must handle Result and a major release is expected, though library-produced digests remain canonical.

Overview
Addresses the V12 audit with breaking security and API hardening across hashing, serialization, and docs/tests.

Hashing / DoS: hash_bytes and hash_squeeze_twice no longer build a full serialized preimage on the heap; they stream the injective encoding via new bytes_to_felts_iter / bytes_to_u64s_iter into the sponge (O(1) extra heap, same digests). Integration tests use a tracking allocator to enforce that.

Collision / validation at decode boundaries: bytes_to_digest, bytes_to_felts_compact, and rehash_to_bytes now return Result and reject 8-byte limbs with value >= P. u64s_to_digest returns Result and rejects limbs wider than 32 bits instead of truncating. u128_to_quantized_felt is replaced by try_u128_to_quantized_felt (no panic on oversized amounts).

Timing / docs: ct_bench wraps measured work in black_box; README and CONSTANT_TIME_TESTING.md narrow constant-time claims to empirical dudect evidence (Goldilocks arithmetic is not strictly branch-free).

New regression tests cover allocation bounds, non-canonical limbs, digest truncation, and quantized amounts; changelog documents unreleased breaking changes.

^{Reviewed by Cursor Bugbot for commit 062e827. Configure here.}

[n13]

Reviewed each finding fix against the audit claims, with independent local verification. The security changes are all correct and well-executed — the one thing that needs fixing before merge is that the new alloc_bounds test is flaky and will intermittently fail CI.

What I verified

• Streaming encoder is byte-identical to the old one. I wrote a differential test pinning the pre-PR bytes_to_u64s algorithm and compared it against both bytes_to_u64s and bytes_to_u64s_iter for every input length 0..=1024 with pseudorandom contents — identical output everywhere, including the empty input and 4-byte-aligned edge cases. The 18 existing KAT vectors also pass, so digests are unchanged.
• Canonicality rejection is sound and can't reject library output. digest_to_bytes serializes as_canonical_u64(), so every library-produced digest has limbs < P and rehash_to_bytes/bytes_to_digest round-trips are infallible for well-behaved callers, exactly as documented. The rejected all-0xFF KAT vector was correctly converted into a rejection test.
• u64s_to_digest / try_u128_to_quantized_felt match the adjacent conventions (as_32_bit_limb_u64 reuse, mirror of try_felt_to_quantized_u128). Panic path is gone.
• Goldilocks branch claims in the docs are accurate — add/sub/reduce128 do contain rare value-dependent correction branches (branch_hint), so narrowing the README claim to empirical dudect evidence is the honest fix; a branchless rewrite would have been riskier.
• No perf regression: hash_only/hash_bytes/8192 measured 266.9µs on this branch vs 273.2µs on main (same machine, within noise).
• Hygiene: full suite passes single-threaded (72 tests), cargo clippy --workspace --all-targets --all-features --release -- -D warnings clean, no hidden/bidi unicode in any changed file.

Blocking: alloc_bounds heap test is flaky

hash_squeeze_twice_uses_constant_heap failed on my first local cargo test run:

peak heap growth was 9757 bytes   (budget: 4096)

Reproduction rates on an M-series mac: 4/50 runs (debug, default test threads), 4/100 (release, default threads), 0/100 with --test-threads=1. CI runs cargo test --locked --release with default threads on two OSes, so at ~4% per binary run this will produce regular spurious failures.

Root cause: MEASURE_LOCK serializes the two test bodies, but libtest still spawns both test threads concurrently — harness bookkeeping and the blocked thread's setup allocate while the first test is measuring, and the global PEAK_ALLOCATED counter attributes that to the measured closure. The comment on MEASURE_LOCK anticipates exactly this failure mode; the lock just can't fully prevent it.

Two options, either is fine:

1. Min over N runs (small diff, verified locally — 0 failures in 200 release runs with default threads):

/// Runs `f` and returns its result plus the peak heap growth (in bytes) observed while it ran.
/// Caller must hold `MEASURE_LOCK`. Takes the minimum over several runs: `f` is deterministic,
/// so the minimum converges to its true peak while filtering out concurrent allocations from
/// libtest harness threads.
fn measure_peak_delta<T>(f: impl Fn() -> T) -> (T, usize) {
	let mut best_peak = usize::MAX;
	let mut result = None;
	for _ in 0..5 {
		let baseline = CURRENT_ALLOCATED.load(SeqCst);
		PEAK_ALLOCATED.store(baseline, SeqCst);
		result = Some(f());
		let peak = PEAK_ALLOCATED.load(SeqCst);
		best_peak = best_peak.min(peak.saturating_sub(baseline));
	}
	(result.expect("ran at least once"), best_peak)
}

2. Thread-scoped tracking — a thread-local flag set only inside measure_peak_delta so the allocator ignores other threads entirely. Deterministic, slightly more code.

Non-blocking notes

• BytesToU64sIter has no size_hint, so bytes_to_u64s/bytes_to_felts lost the exact with_capacity pre-allocation the old implementation had and now grow the Vec geometrically. The output length is exactly (input.len() + 1).div_ceil(4) (wait — +1 for the terminator, padded), so an ExactSizeIterator impl is trivial and restores the old allocation behavior for the collecting APIs.
• Downstream coordination: Quantus-Network/chain primitives/wormhole calls rehash_to_bytes in derive_wormhole_address, which extract_author_from_digest feeds with the miner-supplied 32-byte PreRuntime preimage. On upgrade, that call site must decide explicitly what a rejected non-canonical preimage means (skip attribution? invalid block?) — it's a consensus-visible behavior change from the current silent aliasing, which is precisely the audit issue. Probability of rejecting a random preimage is ~2^-30, so honest-miner impact is negligible, but the error path needs to exist and be deliberate.
• Version is still 2.1.0 with an Unreleased changelog section — consistent with the release-proposal workflow, just remember this needs the major bump option when cutting the release.

Verdict: request changes for the test flake only; with alloc_bounds de-flaked this is ready to merge.

[n13]

Re-reviewed the three commits since my last review (20721be, e4ac1e9, 84f9d90). All points addressed — approving.

Blocking item resolved: alloc_bounds flake

measure_peak_delta now takes the minimum peak over 5 runs. Re-ran the same stress setup that reproduced the flake before:

Configuration	Before	After
release, default test threads	4/100 failures	0/200
debug, default test threads	4/50 failures	0/100

Consider it de-flaked.

size_hint / ExactSizeIterator

Verified correct: re-ran my differential test against the pinned pre-PR bytes_to_u64s algorithm for every input length 0..=1024 — output still byte-identical and iter.len() matches the old vector length everywhere. The exactness invariants ((len+1).div_ceil(4), stays exact while consuming, terminator-word accounting with pos never advancing past the last full chunk) all check out, and the new unit test covers the edge lengths. Collecting APIs get their exact pre-allocation back.

New bytes_to_digest_lossy

Reasonable escape hatch: behavior is exactly the pre-PR bytes_to_digest, the non-injectivity is loudly documented with the correct aliasing probability (~2^-32 per limb), the aliasing is pinned by a test, and the strict decoder remains the default — rehash_to_bytes still goes through the strict path, so the wormhole derivation boundary keeps its canonicality check.

One caution for the downstream migration (chain, zk-circuits): the lossy variant should be reserved for its stated purpose — binding genuinely non-field data (e.g. Blake2 roots) into a Poseidon preimage. A blanket bytes_to_digest(x) → bytes_to_digest_lossy(x) substitution to avoid Result handling would silently reintroduce finding #96398 at that call site. Worth watching for in the dependent-repo PRs.

Re-verified on the new head

• 75 tests pass (62 unit + 11 integration + 2 doc), KAT vectors unchanged
• cargo clippy --workspace --all-targets --all-features --release -- -D warnings clean
• CI green on both OSes
• Still pending at release time: this needs the major bump option in the release workflow.

[illuzen]

v12 pls audit

[v12-auditor]

Warning

V12 could not find a connected workspace for this repository. Make sure the repository owner has signed in to V12 and connected their GitHub account (or that the V12 GitHub App is installed for the owning organization), then comment again.