From 9c9b61f4537e81f50c9886987c1195dd7d012d8b Mon Sep 17 00:00:00 2001 From: MauroFab Date: Mon, 6 Jul 2026 15:50:28 -0300 Subject: [PATCH] fix(stark): reject truncated deep_poly_openings instead of panicking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reconstruct_deep_composition_poly_evaluations_for_all_queries indexes deep_poly_openings[i] for every FRI query index (0..fri_number_of_queries), but that Vec's length is never pinned — the only length guard in the verify path checks the separate query_list field. A malicious proof that keeps query_list intact but truncates deep_poly_openings makes the verifier panic with an out-of-bounds index instead of returning false (verifier DoS). Add a length guard at the top of the reconstruct helper (it already returns Option, so None cleanly rejects), mirroring the existing query_list guard. Add a negative test that truncates deep_poly_openings and asserts the verifier rejects without panicking. --- crypto/stark/src/tests/small_trace_tests.rs | 28 +++++++++++++++++++++ crypto/stark/src/verifier.rs | 12 +++++++++ 2 files changed, 40 insertions(+) diff --git a/crypto/stark/src/tests/small_trace_tests.rs b/crypto/stark/src/tests/small_trace_tests.rs index 96e04858d..ea8d3bc4a 100644 --- a/crypto/stark/src/tests/small_trace_tests.rs +++ b/crypto/stark/src/tests/small_trace_tests.rs @@ -116,6 +116,34 @@ fn test_verify_rejects_truncated_composition_poly_parts_ood() { ); } +/// A malformed proof whose `deep_poly_openings` Vec is shorter than the FRI +/// query count. `reconstruct_deep_composition_poly_evaluations_for_all_queries` +/// indexes `deep_poly_openings[i]` for every query index, and this Vec's length +/// is not otherwise bound (the `query_list.len()` guard checks a different +/// field), so a truncated `deep_poly_openings` must make the verifier return +/// `false` instead of panicking with an out-of-bounds index in release builds. +#[test_log::test] +fn test_verify_rejects_truncated_deep_poly_openings() { + let (air, mut proof) = make_valid_simple_proof(); + + assert!( + proof.deep_poly_openings.len() >= 2, + "test precondition: a valid proof has one deep-poly opening per FRI query", + ); + // Drop the last opening so the Vec is shorter than `fri_number_of_queries`; + // the query loop would then index past the end. + proof.deep_poly_openings.pop(); + + assert!( + !Verifier::verify( + &proof, + &air, + &mut DefaultTranscript::::new(&[]) + ), + "Verifier must reject when deep_poly_openings is shorter than the query count" + ); +} + /// A malformed proof whose deep-poly opening `evaluations` slice has the /// wrong number of columns. The runtime width-mismatch guard added in this /// PR must cause the verifier to return `false` instead of indexing past diff --git a/crypto/stark/src/verifier.rs b/crypto/stark/src/verifier.rs index 616732e22..a9dc8f381 100644 --- a/crypto/stark/src/verifier.rs +++ b/crypto/stark/src/verifier.rs @@ -539,6 +539,18 @@ pub trait IsStarkVerifier< proof: &StarkProof, ) -> Option> { let num_queries = challenges.iotas.len(); + + // `deep_poly_openings` comes straight from the untrusted proof and its + // length is not otherwise pinned (the `query_list.len()` guard checks a + // different field). The loop below indexes `deep_poly_openings[i]` for + // every `i` in `0..num_queries`, so a truncated Vec would panic the + // verifier with an out-of-bounds index on a malicious proof. Reject + // instead. (Extra entries are harmless — they are never indexed — + // matching the `<` convention of the `query_list` guard.) + if proof.deep_poly_openings.len() < num_queries { + return None; + } + let mut deep_poly_evaluations = Vec::with_capacity(num_queries); let mut deep_poly_evaluations_sym = Vec::with_capacity(num_queries);