fix: replace BB_ASSERT with graceful failures in verifier code paths#22311
Draft
AztecBot wants to merge 1 commit intomerge-train/barretenbergfrom
Draft
fix: replace BB_ASSERT with graceful failures in verifier code paths#22311AztecBot wants to merge 1 commit intomerge-train/barretenbergfrom
AztecBot wants to merge 1 commit intomerge-train/barretenbergfrom
Conversation
Convert BB_ASSERT/BB_ASSERT_EQ/BB_ASSERT_LTE calls in verifier code paths to return false or throw_or_abort instead of panicking. Malformed proofs should cause verification to fail gracefully, not crash the process via SIGABRT. Changes by area: - IPA (ipa.hpp): 5 assertions in reduce_verify_internal_native, batch_reduce_verify, and reduce_verify_internal_recursive now return false on invalid input instead of asserting - Translator verifier: 4 size-check assertions now return ReductionResult with reduction_succeeded=false - KZG: MSM size assertion converted to info() warning - Shplonk: constructor assertion converted to explicit throw_or_abort - Shplemini: 2 assertions converted to graceful handling - Proof compression: 11 assertions in decompress_chonk_proof converted to explicit throw_or_abort with descriptive messages - ChonkProof: from_field_elements assertion converted to throw_or_abort - Transcript: 2 bounds-check assertions in receive_from_prover and deserialize_from_buffer converted to explicit throw_or_abort
816473c to
d026957
Compare
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.
Summary
Converts BB_ASSERT calls in verifier code paths to graceful failures instead of panicking. When processing attacker-controlled proof data, malformed proofs should cause verification to return
false(or throw a catchable exception), not crash the process viaSIGABRT.This addresses the broader hardening from the ChonkBatchVerifier security analysis — while the try/catch in
batch_checkalready prevents crashes, these changes make the intent explicit at each verification function: invalid proofs → return false.Changes by area
Bool-returning verification functions → return false:
ipa.hpp): 5 assertions converted —reduce_verify_internal_native(G_zero mismatch),batch_reduce_verify(size/empty checks),reduce_verify_internal_recursive(SRS size, G_zero mismatch)ReductionResult{..., false}Non-bool functions → explicit throw_or_abort or info():
info()warning (optional validation, non-fatal)throw_or_abort(can't return from ctor)if constexpr+ early return; commitment mismatch →info()warningDeserialization/parsing → explicit throw_or_abort:
proof_compression.hpp): All 11 BB_ASSERT calls indecompress_chonk_proofand helpers converted tothrow_or_abortwith descriptive messageschonk_proof.cpp):from_field_elementssize check convertedtranscript.hpp): 2 bounds-check assertions inreceive_from_proveranddeserialize_from_bufferconvertedTest plan
chonk_tests— ProofCompressionRoundtrip + all rejection tests passultra_honk_tests— 112 passcommitment_schemes_tests— All IPA tests passtranslator_vm_tests— builds successfullyClaudeBox log: https://claudebox.work/s/6a1ba966035b66be?run=3