Reject malformed verifier inputs instead of panicking / misverifying#4
Open
aryaethn wants to merge 1 commit into
Open
Reject malformed verifier inputs instead of panicking / misverifying#4aryaethn wants to merge 1 commit into
aryaethn wants to merge 1 commit into
Conversation
Two verifier-entrypoint hardening fixes from the audit (succinctlabs#1): - verify_core now rejects a non-identity C_0 with VerifyError::NonIdentityC0. The pipeline assumes the circuit-R1CS shape C = I (the c-claim is built as a direct z-claim); c0_is_identity() already existed but was only used by the prover, while the verifier assumed it without checking. No-op for all real instances, which are C = I. - verify_claims_ligerito maps verifier_config_for's error into a structured VerifyError::UnsupportedConfig instead of .expect()-panicking, so a bundle claiming an (m, profile) with no embedded security config is rejected rather than crashing the verifier. Adds regression tests for both. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SipengXie2024
added a commit
to SipengXie2024/flock
that referenced
this pull request
Jul 2, 2026
Review findings (36 agents, 86 candidates, 15 confirmed): Fixed: - succinctlabs#7: SSE2 convert-table dispatch missing in s_hat_v partial path (copy-paste omission — x86 fell back to scalar in boundary window) - succinctlabs#8/#12: multi_base.rs imported route.rs (FANOUT=4) instead of route_f32.rs (FANOUT=32). Any fanout>4 would panic. Fixed import + updated acceptance test to use RouteF32Witness. Skipped (documented, not fixable without larger changes): - succinctlabs#1/succinctlabs#3/succinctlabs#6: route_single.rs 3 soundness gaps (documented in header, needs wiring sumcheck) - succinctlabs#2/succinctlabs#4/succinctlabs#9/succinctlabs#10: files >700 LOC (route 810, route_f32 936, route_single 950 — needs structural refactor, separate PR) - succinctlabs#5: multi_base missing padding skip (perf-only, not correctness) - succinctlabs#11: PaddingSpec inconsistency in hash_only PCS open (no effect — padding skip only applies to zerocheck, not PCS) - #13/#14/#15: Ligerito floor magic number, PCS PaddingSpec, target_feature annotation — all by-design or no-impact 338 tests pass (279 flock-core + 50 mhot unit + 8 acceptance + 1 profile).
SipengXie2024
added a commit
to SipengXie2024/flock
that referenced
this pull request
Jul 2, 2026
Review findings (36 agents, 86 candidates, 15 confirmed): Fixed: - succinctlabs#7: SSE2 convert-table dispatch missing in s_hat_v partial path (copy-paste omission — x86 fell back to scalar in boundary window) - succinctlabs#8/#12: multi_base.rs imported route.rs (FANOUT=4) instead of route_f32.rs (FANOUT=32). Any fanout>4 would panic. Fixed import + updated acceptance test to use RouteF32Witness. Skipped (documented, not fixable without larger changes): - succinctlabs#1/succinctlabs#3/succinctlabs#6: route_single.rs 3 soundness gaps (documented in header, needs wiring sumcheck) - succinctlabs#2/succinctlabs#4/succinctlabs#9/succinctlabs#10: files >700 LOC (route 810, route_f32 936, route_single 950 — needs structural refactor, separate PR) - succinctlabs#5: multi_base missing padding skip (perf-only, not correctness) - succinctlabs#11: PaddingSpec inconsistency in hash_only PCS open (no effect — padding skip only applies to zerocheck, not PCS) - #13/#14/#15: Ligerito floor magic number, PCS PaddingSpec, target_feature annotation — all by-design or no-impact 338 tests pass (279 flock-core + 50 mhot unit + 8 acceptance + 1 profile).
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
1 participant
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.
Summary
Two small verifier-entrypoint hardening fixes from the audit (#1) — "verifier panic risks" and "generic R1CS assumes C = I". Both make the verifier reject malformed or unsupported input with a structured error instead of misbehaving.
1. Reject non-identity
C_0verify_corebuilds the c-claim as a direct z-claim, which is only correct for the circuit-R1CS shapeC = I.BlockR1cs::c0_is_identity()already exists and the prover branches on it, but the verifier assumedC = Iwith no check — so an instance withC_0 != Iwould be checked against the wrong relation. Added a guard up front returningVerifyError::NonIdentityC0. No-op for every production instance (allC = I).2. Don't panic on an unsupported Ligerito config
verify_claims_ligeritoderived its security config viaverifier_config_for(...).expect("Ligerito default verifier config").verifier_config_forreturnsErrfor an(m, profile)with no embedded security config — and in the chain path that triple comes from the proof bundle'scommitment.params— so a malformed bundle would panic the verifier. Now mapped toVerifyError::UnsupportedConfig(_)and returned.Tests
verify_core_rejects_non_identity_c0: a zero-C_0instance is rejected withNonIdentityC0(junk sub-proofs are fine — the guard fires before they're touched).verify_claims_ligerito_rejects_unsupported_config: a commitment withm = 8(no embedded config) yieldsUnsupportedConfiginstead of panicking.Note
This is the transcript-independent half of those two audit items. The deeper part of the Ligerito params item — the verifier currently trusts the proof-supplied
profilefor its security schedule, so a downgrade isn't prevented yet — needs a trust-model decision (enforce an expected/minimum profile) and is left for a follow-up.