Skip to content

Commit 2c6023a

Browse files
authored
fix: honk audit fixes (#456)
* fix 1: pin honk_key_gen Solidity VK emission to flavor entity count Audit finding #1. The Solidity VK generator hand-wrote the precomputed-G1 emission list with no link to the flavor, so adding/removing a precomputed entity would silently drift the generated on-chain VK out of sync with the C++ layout. Drive the emission count from a (commitment, name) array and static_assert it against the flavor's size(); add a test asserting the generator emits exactly size() G1 points. * fix 3: set beta_quartic in RelationParameters::compute_beta_powers Audit finding #3. compute_beta_powers() set beta..beta_cube but left beta_quartic at 0, so the test-only get_random() built eccvm_set_permutation_delta with a zero domain-separation tag instead of the production FIRST_TERM_TAG * beta^4. Set beta_quartic = beta_cube * beta so test-infra parameters match the eccvm relation semantics. * fix 4: reject wrong-size field input in VK deserialization Audit finding #4. NativeVerificationKey_::from_field_elements and the StdlibVerificationKey_ span constructor consumed only the fields they needed and silently ignored trailing ones, so an oversize (malformed) VK deserialized without error. Assert the input field count matches the VK layout exactly on both paths so a malformed VK is rejected rather than masked. * fix 6: add [[nodiscard]] to create_recursion_constraints Audit finding #6. create_recursion_constraints returns the aggregated pairing points and IPA claim the caller must deferred-accumulate to complete recursive verification, but unlike its inner helper create_honk_recursion_constraints it carried no [[nodiscard]] -- a caller could silently drop the result and skip accumulation with no diagnostic. Add the attribute, matching the inner helper. * fix 7: assert that hasZK and masking layout are consistent. * fix: extend gemini-masking-layout assert to remaining ZK flavors Finding #7's gemini_masking_layout_consistent<> static_assert was only on Ultra/UltraZK/Mega/MegaZKFlavor. UltraKeccakZKFlavor, UltraZKRecursiveFlavor and MegaZKRecursiveFlavor hand-declare both the masking flag and the AllValues layout, so they can drift independently the same way -- MegaZKRecursiveFlavor most acutely, since its HasGeminiMasking=false is decoupled from HasZK=true. Add the assert to all three; all are currently consistent, so this is pure hardening.
1 parent 3e2fec3 commit 2c6023a

16 files changed

Lines changed: 214 additions & 31 deletions

barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,8 @@ struct RecursionConstraint {
132132
* @param chonk_recursion_data pair of (ChonkRecursionConstraints, ChonkRecursionConstraintsOriginalOpcodeIndices)
133133
*/
134134
template <typename Builder>
135-
HonkRecursionConstraintsOutput<Builder> create_recursion_constraints(
135+
[[nodiscard("pairing points and IPA claim must be accumulated")]] HonkRecursionConstraintsOutput<Builder>
136+
create_recursion_constraints(
136137
Builder& builder,
137138
GateCounter<Builder>& gate_counter,
138139
std::vector<size_t>& gates_per_opcode,

barretenberg/cpp/src/barretenberg/flavor/flavor.hpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,9 @@ class NativeVerificationKey_ : public PrecomputedCommitments {
238238
*/
239239
size_t from_field_elements(const std::span<const DataType>& elements)
240240
{
241+
BB_ASSERT_EQ(elements.size(),
242+
calc_num_data_types(),
243+
"VerificationKey::from_field_elements received the wrong number of field elements");
241244

242245
size_t idx = 0;
243246
auto deserialize = [&idx, &elements]<typename T>(T& target) {
@@ -418,6 +421,9 @@ class StdlibVerificationKey_ : public PrecomputedCommitments {
418421
for (Commitment& commitment : this->get_all()) {
419422
commitment = Codec::template deserialize_from_frs<Commitment>(elements, num_frs_read);
420423
}
424+
BB_ASSERT_EQ(num_frs_read,
425+
elements.size(),
426+
"StdlibVerificationKey deserialization received the wrong number of field elements");
421427
}
422428

423429
/**

barretenberg/cpp/src/barretenberg/flavor/flavor_concepts.hpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,5 +107,21 @@ constexpr bool flavor_has_gemini_masking()
107107
}
108108
}
109109

110+
// Whether a flavor's entity layout actually carries a gemini_masking_poly column.
111+
template <typename Flavor>
112+
constexpr bool flavor_entities_have_gemini_masking()
113+
{
114+
return requires(typename Flavor::AllValues values) { values.gemini_masking_poly(); };
115+
}
116+
117+
// The masking invariant: the advertised flag must match the real entity layout. A flavor that
118+
// declares Gemini masking (via HasGeminiMasking / HasZK) must actually carry the gemini_masking_poly
119+
// column, and vice versa.
120+
template <typename Flavor>
121+
constexpr bool gemini_masking_layout_consistent()
122+
{
123+
return flavor_has_gemini_masking<Flavor>() == flavor_entities_have_gemini_masking<Flavor>();
124+
}
125+
110126
// clang-format on
111127
} // namespace bb

barretenberg/cpp/src/barretenberg/flavor/mega_flavor.hpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "barretenberg/commitment_schemes/kzg/kzg.hpp"
1010
#include "barretenberg/flavor/flavor.hpp"
11+
#include "barretenberg/flavor/flavor_concepts.hpp"
1112
#include "barretenberg/flavor/generated/mega_flavor_generated.hpp"
1213
#include "barretenberg/flavor/partially_evaluated_multivariates.hpp"
1314
#include "barretenberg/flavor/prover_polynomials.hpp"
@@ -88,6 +89,9 @@ class MegaFlavor : public MegaFlavor_Generated {
8889
*/
8990
using AllValues = AllEntities<FF>;
9091

92+
static_assert(gemini_masking_layout_consistent<MegaFlavor>(),
93+
"MegaFlavor gemini masking flag must match its entity layout");
94+
9195
/**
9296
* @brief A container for the prover polynomials handles.
9397
*/

barretenberg/cpp/src/barretenberg/flavor/mega_zk_flavor.hpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa_utils.hpp"
1111
#include "barretenberg/constants.hpp"
1212
#include "barretenberg/flavor/flavor.hpp"
13+
#include "barretenberg/flavor/flavor_concepts.hpp"
1314
#include "barretenberg/flavor/generated/mega_zk_flavor_generated.hpp"
1415
#include "barretenberg/flavor/partially_evaluated_multivariates.hpp"
1516
#include "barretenberg/flavor/prover_polynomials.hpp"
@@ -103,6 +104,10 @@ class MegaZKFlavor : public MegaZKFlavor_Generated {
103104
}
104105

105106
using AllValues = AllEntities<FF>;
107+
108+
static_assert(gemini_masking_layout_consistent<MegaZKFlavor>(),
109+
"MegaZKFlavor gemini masking flag must match its entity layout");
110+
106111
using ProverPolynomials = ProverPolynomialsBase<AllEntities<Polynomial>, AllValues, Polynomial>;
107112
using PrecomputedData = PrecomputedData_<Polynomial, NUM_PRECOMPUTED_ENTITIES>;
108113
using VerificationKey = NativeVerificationKey_<PrecomputedEntities<Commitment>, Codec, HashFunction, CommitmentKey>;

barretenberg/cpp/src/barretenberg/flavor/mega_zk_recursive_flavor.hpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#pragma once
88
#include "barretenberg/commitment_schemes/kzg/kzg.hpp"
9+
#include "barretenberg/flavor/flavor_concepts.hpp"
910
#include "barretenberg/flavor/mega_zk_flavor.hpp"
1011
#include "barretenberg/stdlib/primitives/curves/bn254.hpp"
1112
#include "barretenberg/stdlib/primitives/field/field.hpp"
@@ -78,6 +79,9 @@ template <typename BuilderType> class MegaZKRecursiveFlavor_ {
7879
using Base::Base;
7980
};
8081

82+
static_assert(gemini_masking_layout_consistent<MegaZKRecursiveFlavor_>(),
83+
"MegaZKRecursiveFlavor gemini masking flag must match its entity layout");
84+
8185
using VerificationKey = StdlibVerificationKey_<CircuitBuilder,
8286
NativeFlavor::PrecomputedEntities<Commitment>,
8387
NativeFlavor::VerificationKey>;

barretenberg/cpp/src/barretenberg/flavor/native_verification_key.test.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,3 +88,21 @@ TYPED_TEST(NativeVerificationKeyTests, VKSizeCheck)
8888
VerificationKey vk(TestFixture::create_vk());
8989
EXPECT_EQ(vk.to_field_elements().size(), VerificationKey::calc_num_data_types());
9090
}
91+
92+
// from_field_elements must require an exact field count, not just enough fields.
93+
TYPED_TEST(NativeVerificationKeyTests, FromFieldElementsRejectsWrongSize)
94+
{
95+
using Flavor = typename TypeParam::Flavor;
96+
using VerificationKey = typename Flavor::VerificationKey;
97+
98+
VerificationKey vk(TestFixture::create_vk());
99+
const auto fields = vk.to_field_elements();
100+
101+
VerificationKey roundtrip;
102+
EXPECT_NO_THROW(roundtrip.from_field_elements(fields));
103+
104+
auto oversize = fields;
105+
oversize.push_back(fields.back());
106+
VerificationKey bad;
107+
EXPECT_ANY_THROW(bad.from_field_elements(oversize));
108+
}

barretenberg/cpp/src/barretenberg/flavor/stdlib_verification_key.test.cpp

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,43 @@ TYPED_TEST(StdlibVerificationKeyTests, VKHashingConsistency)
9090
FF vk_hash_2 = vk.hash_with_origin_tagging(transcript);
9191
EXPECT_EQ(vk_hash_1.get_value(), vk_hash_2.get_value());
9292
}
93+
94+
// The span constructor must require an exact field count, not just enough fields.
95+
TYPED_TEST(StdlibVerificationKeyTests, SpanConstructorRejectsWrongSize)
96+
{
97+
using Flavor = TypeParam;
98+
using NativeFlavor = typename Flavor::NativeFlavor;
99+
using NativeVerificationKey = typename NativeFlavor::VerificationKey;
100+
using StdlibVerificationKey = typename Flavor::VerificationKey;
101+
using OuterBuilder = typename Flavor::CircuitBuilder;
102+
using FF = stdlib::field_t<OuterBuilder>;
103+
using Codec = stdlib::StdlibCodec<FF>;
104+
using ProverInstance = ProverInstance_<NativeFlavor>;
105+
using InnerBuilder = typename NativeFlavor::CircuitBuilder;
106+
107+
InnerBuilder builder;
108+
stdlib::recursion::honk::DefaultIO<InnerBuilder>::add_default(builder);
109+
auto prover_instance = std::make_shared<ProverInstance>(builder);
110+
auto native_vk = std::make_shared<NativeVerificationKey>(prover_instance->get_precomputed());
111+
112+
OuterBuilder outer_builder;
113+
StdlibVerificationKey vk(&outer_builder, native_vk);
114+
115+
std::vector<FF> fields;
116+
auto append = [&]<typename T>(const T& input) {
117+
std::vector<FF> input_fields = Codec::template serialize_to_fields<T>(input);
118+
fields.insert(fields.end(), input_fields.begin(), input_fields.end());
119+
};
120+
append(vk.log_circuit_size);
121+
append(vk.num_public_inputs);
122+
append(vk.pub_inputs_offset);
123+
for (const auto& commitment : vk.get_all()) {
124+
append(commitment);
125+
}
126+
127+
EXPECT_NO_THROW(StdlibVerificationKey{ std::span<FF>(fields) });
128+
129+
std::vector<FF> oversize = fields;
130+
oversize.push_back(fields.back());
131+
EXPECT_ANY_THROW(StdlibVerificationKey{ std::span<FF>(oversize) });
132+
}

barretenberg/cpp/src/barretenberg/flavor/ultra_flavor.hpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "barretenberg/commitment_schemes/kzg/kzg.hpp"
1010
#include "barretenberg/flavor/flavor.hpp"
11+
#include "barretenberg/flavor/flavor_concepts.hpp"
1112
#include "barretenberg/flavor/generated/ultra_flavor_generated.hpp"
1213
#include "barretenberg/flavor/generated/ultra_zk_flavor_generated.hpp"
1314
#include "barretenberg/flavor/partially_evaluated_multivariates.hpp"
@@ -115,6 +116,9 @@ class UltraFlavor : public UltraFlavor_Generated {
115116

116117
using AllValues = AllValues_<HasZK>;
117118

119+
static_assert(gemini_masking_layout_consistent<UltraFlavor>(),
120+
"UltraFlavor gemini masking flag must match its entity layout");
121+
118122
/**
119123
* @brief A container for polynomials handles.
120124
*/

barretenberg/cpp/src/barretenberg/flavor/ultra_keccak_zk_flavor.hpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "barretenberg/commitment_schemes/small_subgroup_ipa/small_subgroup_ipa_utils.hpp"
1010
#include "barretenberg/constants.hpp"
11+
#include "barretenberg/flavor/flavor_concepts.hpp"
1112
#include "barretenberg/flavor/ultra_keccak_flavor.hpp"
1213

1314
namespace bb {
@@ -44,6 +45,10 @@ class UltraKeccakZKFlavor : public UltraKeccakFlavor {
4445
}
4546

4647
using AllValues = UltraFlavor::AllValues_<HasZK>;
48+
49+
static_assert(gemini_masking_layout_consistent<UltraKeccakZKFlavor>(),
50+
"UltraKeccakZKFlavor gemini masking flag must match its entity layout");
51+
4752
using ProverPolynomials = UltraFlavor::ProverPolynomials_<HasZK>;
4853
using PartiallyEvaluatedMultivariates = UltraFlavor::PartiallyEvaluatedMultivariates_<HasZK>;
4954

0 commit comments

Comments
 (0)