Skip to content

Commit 51c63db

Browse files
authored
feat: merge-train/barretenberg (#22324)
BEGIN_COMMIT_OVERRIDE fix: reject VK with log_circuit_size=0 in UltraKeccak verifier (#22319) chore: merkle tree audit 2 (#21475) fix: graceful failures in verifier code paths + other fixes (#22311) fix: Fr::from_u64 big-endian encoding to match C++ msgpack format (#22233) fix: corrupt low-order bytes in batch verifier test to avoid non-canonical field encoding (#22333) fix: skip MsgpackRejectsNonCanonical test in WASM builds (#22335) END_COMMIT_OVERRIDE
2 parents f6cfe1a + 682e928 commit 51c63db

38 files changed

Lines changed: 340 additions & 93 deletions

barretenberg/cpp/scripts/audit/audit_scopes/merkle_tree_audit_scope.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# External Audit Scope: merkle_tree
22

33
Repository: https://github.com/AztecProtocol/aztec-packages
4-
Commit hash: Most recent commit on branch 'next'
4+
Commit hash: 22d6fc368da0fbe5412f4f7b2890a052aa48d803
55

66
## Files to Audit
77
Note: Paths relative to `aztec-packages/barretenberg/cpp/src/barretenberg`

barretenberg/cpp/src/barretenberg/bbapi/bbapi_chonk.cpp

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,14 @@ ChonkStart::Response ChonkStart::execute(BBApiRequest& request) &&
2828
BB_BENCH_NAME(MSGPACK_SCHEMA_NAME);
2929

3030
request.ivc_in_progress = std::make_shared<Chonk>(num_circuits);
31-
3231
request.ivc_stack_depth = 0;
32+
33+
// Clear any stale loaded-circuit state from a previous session so that
34+
// ChonkAccumulate cannot silently reuse a circuit loaded before this ChonkStart.
35+
request.loaded_circuit_name.clear();
36+
request.loaded_circuit_constraints.reset();
37+
request.loaded_circuit_vk.clear();
38+
3339
return Response{};
3440
}
3541

@@ -63,6 +69,15 @@ ChonkAccumulate::Response ChonkAccumulate::execute(BBApiRequest& request) &&
6369
acir_format::WitnessVector witness_data = acir_format::witness_buf_to_witness_vector(std::move(witness));
6470
acir_format::AcirProgram program{ std::move(request.loaded_circuit_constraints.value()), std::move(witness_data) };
6571

72+
// Clear loaded state immediately after moving out of it. This ensures that if any subsequent
73+
// step throws, the request won't appear to still have a valid circuit loaded (the optional
74+
// would be in a moved-from state, which is technically has_value()==true but poisoned).
75+
auto loaded_vk = std::move(request.loaded_circuit_vk);
76+
auto circuit_name = std::move(request.loaded_circuit_name);
77+
request.loaded_circuit_constraints.reset();
78+
request.loaded_circuit_vk.clear();
79+
request.loaded_circuit_name.clear();
80+
6681
const acir_format::ProgramMetadata metadata{ .ivc = request.ivc_in_progress };
6782
auto circuit = acir_format::create_circuit<IVCBase::ClientCircuit>(program, metadata);
6883

@@ -71,17 +86,17 @@ ChonkAccumulate::Response ChonkAccumulate::execute(BBApiRequest& request) &&
7186
if (request.vk_policy == VkPolicy::RECOMPUTE) {
7287
precomputed_vk = nullptr;
7388
} else if (request.vk_policy == VkPolicy::DEFAULT || request.vk_policy == VkPolicy::CHECK) {
74-
if (!request.loaded_circuit_vk.empty()) {
75-
validate_vk_size<Chonk::MegaVerificationKey>(request.loaded_circuit_vk);
76-
precomputed_vk = from_buffer<std::shared_ptr<Chonk::MegaVerificationKey>>(request.loaded_circuit_vk);
89+
if (!loaded_vk.empty()) {
90+
validate_vk_size<Chonk::MegaVerificationKey>(loaded_vk);
91+
precomputed_vk = from_buffer<std::shared_ptr<Chonk::MegaVerificationKey>>(loaded_vk);
7792

7893
if (request.vk_policy == VkPolicy::CHECK) {
7994
auto prover_instance = std::make_shared<Chonk::ProverInstance>(circuit);
8095
auto computed_vk = std::make_shared<Chonk::MegaVerificationKey>(prover_instance->get_precomputed());
8196

8297
// Dereference to compare VK contents
8398
if (*precomputed_vk != *computed_vk) {
84-
throw_or_abort("VK check failed for circuit '" + request.loaded_circuit_name +
99+
throw_or_abort("VK check failed for circuit '" + circuit_name +
85100
"': provided VK does not match computed VK");
86101
}
87102
}
@@ -90,16 +105,13 @@ ChonkAccumulate::Response ChonkAccumulate::execute(BBApiRequest& request) &&
90105
throw_or_abort("Invalid VK policy. Valid options: default, check, recompute");
91106
}
92107

93-
info("ChonkAccumulate - accumulating circuit '", request.loaded_circuit_name, "'");
108+
info("ChonkAccumulate - accumulating circuit '", circuit_name, "'");
94109
if (detail::use_memory_profile) {
95-
detail::GLOBAL_MEMORY_PROFILE.set_circuit_name(request.loaded_circuit_name);
110+
detail::GLOBAL_MEMORY_PROFILE.set_circuit_name(circuit_name);
96111
}
97112
request.ivc_in_progress->accumulate(circuit, precomputed_vk);
98113
request.ivc_stack_depth++;
99114

100-
request.loaded_circuit_constraints.reset();
101-
request.loaded_circuit_vk.clear();
102-
103115
return Response{};
104116
}
105117

barretenberg/cpp/src/barretenberg/chonk/batched_honk_translator/batched_honk_translator_verifier.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ typename BatchedHonkTranslatorVerifier_<Curve>::OinkResult BatchedHonkTranslator
4444
mega_zk_verifier_instance = std::make_shared<MegaZKVerifierInstance>(mega_zk_vk_and_hash);
4545

4646
// Derive num_public_inputs from the Oink-only MegaZK proof.
47+
if (mega_zk_proof.size() < ProofLength::Oink<MegaZKFlavorT>::LENGTH_WITHOUT_PUB_INPUTS) {
48+
throw_or_abort("MegaZK Oink proof too short to derive num_public_inputs");
49+
}
4750
const size_t num_public_inputs = mega_zk_proof.size() - ProofLength::Oink<MegaZKFlavorT>::LENGTH_WITHOUT_PUB_INPUTS;
4851

4952
OinkVerifier<MegaZKFlavorT> oink_verifier{ mega_zk_verifier_instance, transcript, num_public_inputs };

barretenberg/cpp/src/barretenberg/chonk/chonk.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,7 @@ void Chonk::update_native_verifier_accumulator(const VerifierInputs& queue_entry
732732
info("Chonk accumulate: hash of verifier accumulator computed natively set in previous kernel IO: ",
733733
native_verifier_accum_hash);
734734
}
735-
has_last_app_been_accumulated = num_circuits_accumulated + 1 == num_circuits - 4;
735+
has_last_app_been_accumulated = num_circuits_accumulated + 1 == num_circuits - 3;
736736
is_previous_circuit_a_kernel = queue_entry.is_kernel;
737737

738738
info("======= END OF DEBUGGING INFO FOR NATIVE FOLDING STEP =======");

barretenberg/cpp/src/barretenberg/chonk/chonk.test.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -449,8 +449,9 @@ TEST_F(ChonkTests, MsgpackProofFromFileOrBuffer)
449449
std::generate(random_bytes.begin(), random_bytes.end(), []() { return static_cast<uint8_t>(rand() % 256); });
450450
std::copy(random_bytes.begin(), random_bytes.end(), buffer.data());
451451

452-
// Expect deserialization to fail with error msgpack::v1::type_error with description "std::bad_cast"
453-
EXPECT_THROW(ChonkProof::from_msgpack_buffer(buffer), msgpack::v1::type_error);
452+
// Expect deserialization to fail (either msgpack parse error, type mismatch, trailing data,
453+
// or non-canonical field encoding)
454+
EXPECT_ANY_THROW(ChonkProof::from_msgpack_buffer(buffer));
454455
}
455456
};
456457

barretenberg/cpp/src/barretenberg/chonk/chonk_proof.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ ChonkProof_<IsRecursive> ChonkProof_<IsRecursive>::from_field_elements(const std
4444
// MegaZK Oink proof size = total - all other fixed-size components.
4545
// This correctly accounts for any ACIR public inputs prepended to the oink portion.
4646
constexpr size_t fixed_total = merge_size + eccvm_size + ipa_size + joint_size;
47-
BB_ASSERT_GTE(fields.size(), fixed_total, "ChonkProof::from_field_elements: proof too short");
47+
if (fields.size() < fixed_total) {
48+
throw_or_abort("ChonkProof::from_field_elements: proof too short");
49+
}
4850
const size_t mega_zk_oink_length = fields.size() - fixed_total;
4951

5052
auto it = fields.begin();

barretenberg/cpp/src/barretenberg/chonk/chonk_proof.hpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,14 @@ template <bool IsRecursive = false> struct ChonkProof_ {
124124
static ChonkProof_ from_msgpack_buffer(const msgpack::sbuffer& buffer)
125125
requires(!IsRecursive)
126126
{
127-
msgpack::object_handle oh = msgpack::unpack(buffer.data(), buffer.size());
128-
msgpack::object obj = oh.get();
127+
std::size_t offset = 0;
128+
msgpack::object_handle oh = msgpack::unpack(buffer.data(), buffer.size(), offset);
129+
if (offset != buffer.size()) {
130+
throw_or_abort("ChonkProof::from_msgpack_buffer: trailing data (" + std::to_string(buffer.size() - offset) +
131+
" extra bytes)");
132+
}
129133
ChonkProof_ proof;
130-
obj.convert(proof);
134+
oh.get().convert(proof);
131135
return proof;
132136
}
133137

barretenberg/cpp/src/barretenberg/chonk/proof_compression.hpp

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,9 @@ class ProofCompressor {
5959

6060
static uint256_t read_u256(const std::vector<uint8_t>& data, size_t& pos)
6161
{
62-
BB_ASSERT(pos + 32 <= data.size());
62+
if (pos + 32 > data.size()) {
63+
throw_or_abort("proof_compression: read_u256 out of bounds");
64+
}
6365
uint256_t val{ 0, 0, 0, 0 };
6466
for (int i = 31; i >= 0; --i) {
6567
val.data[i / 8] |= static_cast<uint64_t>(data[pos++]) << (8 * (i % 8));
@@ -423,10 +425,14 @@ class ProofCompressor {
423425
*/
424426
static size_t compressed_mega_num_public_inputs(size_t compressed_bytes)
425427
{
426-
BB_ASSERT(compressed_bytes % 32 == 0);
428+
if (compressed_bytes % 32 != 0) {
429+
throw_or_abort("proof_compression: compressed size not aligned to 32 bytes");
430+
}
427431
size_t total_elements = compressed_bytes / 32;
428432
size_t fixed_elements = compressed_element_count(0);
429-
BB_ASSERT(total_elements >= fixed_elements);
433+
if (total_elements < fixed_elements) {
434+
throw_or_abort("proof_compression: compressed proof too short");
435+
}
430436
return total_elements - fixed_elements;
431437
}
432438

@@ -493,7 +499,9 @@ class ProofCompressor {
493499
size_t mega_num_pub_inputs =
494500
proof.hiding_oink_proof.size() - ProofLength::Oink<MegaZKFlavor>::LENGTH_WITHOUT_PUB_INPUTS;
495501
walk_chonk_proof(bn254_scalar, bn254_comm, grumpkin_scalar, grumpkin_comm, mega_num_pub_inputs);
496-
BB_ASSERT(offset == flat.size());
502+
if (offset != flat.size()) {
503+
throw_or_abort("proof_compression: compress did not consume all proof elements");
504+
}
497505
return out;
498506
}
499507

@@ -505,7 +513,9 @@ class ProofCompressor {
505513
// BN254 callbacks
506514
auto bn254_scalar = [&]() {
507515
uint256_t raw = read_u256(compressed, pos);
508-
BB_ASSERT(raw < Fr::modulus);
516+
if (raw >= Fr::modulus) {
517+
throw_or_abort("proof_compression: BN254 scalar out of range");
518+
}
509519
flat.emplace_back(raw);
510520
};
511521

@@ -523,11 +533,15 @@ class ProofCompressor {
523533
return;
524534
}
525535

526-
BB_ASSERT(x_val < Fq::modulus);
536+
if (x_val >= Fq::modulus) {
537+
throw_or_abort("proof_compression: BN254 x-coordinate out of range");
538+
}
527539
Fq x(x_val);
528540
Fq y_squared = x * x * x + Bn254G1Params::b;
529541
auto [is_square, y] = y_squared.sqrt();
530-
BB_ASSERT(is_square);
542+
if (!is_square) {
543+
throw_or_abort("proof_compression: BN254 point not on curve");
544+
}
531545

532546
if (y_is_negative(y) != sign) {
533547
y = -y;
@@ -555,11 +569,15 @@ class ProofCompressor {
555569
return;
556570
}
557571

558-
BB_ASSERT(x_val < Fr::modulus);
572+
if (x_val >= Fr::modulus) {
573+
throw_or_abort("proof_compression: Grumpkin x-coordinate out of range");
574+
}
559575
Fr x(x_val);
560576
Fr y_squared = x * x * x + grumpkin::G1Params::b;
561577
auto [is_square, y] = y_squared.sqrt();
562-
BB_ASSERT(is_square);
578+
if (!is_square) {
579+
throw_or_abort("proof_compression: Grumpkin point not on curve");
580+
}
563581

564582
if (y_is_negative(y) != sign) {
565583
y = -y;
@@ -571,15 +589,19 @@ class ProofCompressor {
571589

572590
auto grumpkin_scalar = [&]() {
573591
uint256_t raw = read_u256(compressed, pos);
574-
BB_ASSERT(raw < Fq::modulus);
592+
if (raw >= Fq::modulus) {
593+
throw_or_abort("proof_compression: Grumpkin scalar out of range");
594+
}
575595
Fq fq_val(raw);
576596
auto [lo, hi] = split_fq(fq_val);
577597
flat.emplace_back(lo);
578598
flat.emplace_back(hi);
579599
};
580600

581601
walk_chonk_proof(bn254_scalar, bn254_comm, grumpkin_scalar, grumpkin_comm, mega_num_public_inputs);
582-
BB_ASSERT(pos == compressed.size());
602+
if (pos != compressed.size()) {
603+
throw_or_abort("proof_compression: decompression did not consume all bytes");
604+
}
583605
return ChonkProof::from_field_elements(flat);
584606
}
585607
};

barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,7 @@ template <typename Curve_, size_t log_poly_length = CONST_ECCVM_LOG_N> class IPA
475475
scalar_multiplication::pippenger_unsafe<Curve>(data.s_vec, { &srs_elements[0], /*size*/ poly_length });
476476
}
477477
if (G_zero != data.G_zero_from_prover) {
478+
info("IPA verification failed: G_0 mismatch");
478479
return false;
479480
}
480481

@@ -669,8 +670,14 @@ template <typename Curve_, size_t log_poly_length = CONST_ECCVM_LOG_N> class IPA
669670
requires(!Curve::is_stdlib_type)
670671
{
671672
const size_t num_claims = opening_claims.size();
672-
BB_ASSERT(num_claims == transcripts.size());
673-
BB_ASSERT(num_claims > 0);
673+
if (num_claims != transcripts.size()) {
674+
info("IPA batch verification failed: claims/transcripts size mismatch");
675+
return false;
676+
}
677+
if (num_claims == 0) {
678+
info("IPA batch verification failed: no claims provided");
679+
return false;
680+
}
674681

675682
// Phase 1: Per-proof transcript processing (sequential, each proof is cheap)
676683
std::vector<GroupElement> C_zeros(num_claims);
@@ -770,6 +777,12 @@ template <typename Curve_, size_t log_poly_length = CONST_ECCVM_LOG_N> class IPA
770777
static bool full_verify_recursive(const VK& vk, const OpeningClaim<Curve>& opening_claim, auto& transcript)
771778
requires Curve::is_stdlib_type
772779
{
780+
// Check SRS size up front before any circuit construction
781+
if (vk.get_monomial_points().size() < poly_length) {
782+
throw_or_abort("IPA recursive verification: not enough SRS points (need " + std::to_string(poly_length) +
783+
", have " + std::to_string(vk.get_monomial_points().size()) + ")");
784+
}
785+
773786
add_claim_to_hash_buffer(opening_claim, transcript);
774787
VerifierAccumulator verifier_accumulator = reduce_verify_internal_recursive(opening_claim, transcript);
775788
auto round_challenges_inv = verifier_accumulator.u_challenges_inv;
@@ -806,7 +819,6 @@ template <typename Curve_, size_t log_poly_length = CONST_ECCVM_LOG_N> class IPA
806819
// Compute G_zero
807820
// In the native verifier, this uses pippenger. Here we use batch_mul.
808821
std::vector<Commitment> srs_elements = vk.get_monomial_points();
809-
BB_ASSERT_GTE(srs_elements.size(), poly_length, "Not enough SRS points for IPA!");
810822
srs_elements.resize(poly_length);
811823
Commitment computed_G_zero = Commitment::batch_mul(srs_elements, s_vec);
812824
// check the computed G_zero and the claimed G_zero are the same.

barretenberg/cpp/src/barretenberg/commitment_schemes/kzg/kzg.hpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,11 @@ template <typename Curve_> class KZG {
151151

152152
// Validate the final MSM size if expected size is provided
153153
if (expected_final_msm_size != 0) {
154-
BB_ASSERT_EQ(batch_opening_claim.commitments.size(), expected_final_msm_size);
154+
if (batch_opening_claim.commitments.size() != expected_final_msm_size) {
155+
throw_or_abort("KZG verification: unexpected final MSM size " +
156+
std::to_string(batch_opening_claim.commitments.size()) + " (expected " +
157+
std::to_string(expected_final_msm_size) + ")");
158+
}
155159
}
156160

157161
// Compute C + [W]₁ ⋅ z

0 commit comments

Comments
 (0)