From a2f328ddb68480558700495209e53807759c376c Mon Sep 17 00:00:00 2001 From: dbanks12 Date: Sat, 10 Jan 2026 01:13:56 +0000 Subject: [PATCH] fix!: defensive ghost row constraints in bc_hashing pil --- .../cpp/pil/vm2/bytecode/bc_hashing.pil | 6 +++ .../relations/bc_hashing.test.cpp | 31 +++++++++++++ .../vm2/generated/relations/bc_hashing.hpp | 26 ++++++----- .../generated/relations/bc_hashing_impl.hpp | 44 ++++++++++++------- 4 files changed, 81 insertions(+), 26 deletions(-) diff --git a/barretenberg/cpp/pil/vm2/bytecode/bc_hashing.pil b/barretenberg/cpp/pil/vm2/bytecode/bc_hashing.pil index ccc3f8abd0ca..e7d7eac1d299 100644 --- a/barretenberg/cpp/pil/vm2/bytecode/bc_hashing.pil +++ b/barretenberg/cpp/pil/vm2/bytecode/bc_hashing.pil @@ -136,6 +136,12 @@ namespace bc_hashing; pol commit sel_not_padding_2; // @boolean sel_not_padding_1 * (1 - sel_not_padding_1) = 0; sel_not_padding_2 * (1 - sel_not_padding_2) = 0; + // Force sel_not_padding_1 to 0 when sel is 0 (prevents ghost row injection attacks) + #[SEL_NOT_PADDING_1_REQUIRES_SEL] + sel_not_padding_1 * (1 - sel) = 0; + // Force sel_not_padding_2 to 0 when sel is 0 (prevents ghost row injection attacks) + #[SEL_NOT_PADDING_2_REQUIRES_SEL] + sel_not_padding_2 * (1 - sel) = 0; // TODO: Instead of two bools, change to value = 0 (no padding) OR 1 (one field padding) OR 2 (two fields padding)? // The annoyance is that we need many committed sels to use in/gate lookups. diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/bc_hashing.test.cpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/bc_hashing.test.cpp index 3f06995e8ace..3ea2a46ca766 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/bc_hashing.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/bc_hashing.test.cpp @@ -737,5 +737,36 @@ TEST_F(BytecodeHashingConstrainingTest, NegativeSingleBytecodeHashOutputConsiste "Failed.*CHECK_FINAL_BYTES_REMAINING. Could not find tuple in destination."); EXPECT_THROW_WITH_MESSAGE(check_relation(trace, bc_hashing::SR_ID_PROPAGATION), "ID_PROPAGATION"); } +// ===================================================================== +// Ghost Row Injection Vulnerability Tests +// ===================================================================== +// These tests verify that ghost rows (sel=0) cannot fire permutations. +// The fix: sel_not_padding_1 * (1 - sel) = 0 and sel_not_padding_2 * (1 - sel) = 0 +// ensure these selectors are forced to 0 when sel=0. + +TEST_F(BytecodeHashingConstrainingTest, NegativeGhostRowInjectionBlocked) +{ + // Try to create a ghost row (sel=0) with sel_not_padding_1=1 or sel_not_padding_2=1 + // which would fire the #[GET_PACKED_FIELD_1] or #[GET_PACKED_FIELD_2] permutations + TestTraceContainer trace({ + { { C::precomputed_first_row, 1 } }, + { + { C::bc_hashing_sel, 0 }, // Ghost row: gadget not active + { C::bc_hashing_sel_not_padding_1, 1 }, // Try to fire permutation anyway + { C::bc_hashing_sel_not_padding_2, 0 }, + }, + }); + + // The fix: sel_not_padding_1 * (1 - sel) = 0 + EXPECT_THROW_WITH_MESSAGE(check_relation(trace), "SEL_NOT_PADDING_1_REQUIRES_SEL"); + + // Reset and try with sel_not_padding_2 + trace.set(C::bc_hashing_sel_not_padding_1, 1, 0); + trace.set(C::bc_hashing_sel_not_padding_2, 1, 1); + + // The fix: sel_not_padding_2 * (1 - sel) = 0 + EXPECT_THROW_WITH_MESSAGE(check_relation(trace), "SEL_NOT_PADDING_2_REQUIRES_SEL"); +} + } // namespace } // namespace bb::avm2::constraining diff --git a/barretenberg/cpp/src/barretenberg/vm2/generated/relations/bc_hashing.hpp b/barretenberg/cpp/src/barretenberg/vm2/generated/relations/bc_hashing.hpp index 74f345e7b970..71a57b393a07 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/generated/relations/bc_hashing.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/generated/relations/bc_hashing.hpp @@ -14,8 +14,8 @@ template class bc_hashingImpl { public: using FF = FF_; - static constexpr std::array SUBRELATION_PARTIAL_LENGTHS = { 3, 4, 3, 3, 3, 3, 3, 4, 4, 3, 3, - 3, 3, 3, 4, 4, 4, 4, 5, 3, 4, 3 }; + static constexpr std::array SUBRELATION_PARTIAL_LENGTHS = { 3, 4, 3, 3, 3, 3, 3, 4, 4, 3, 3, 3, + 3, 3, 3, 3, 4, 4, 4, 4, 5, 3, 4, 3 }; template inline static bool skip(const AllEntities& in) { @@ -44,14 +44,16 @@ template class bc_hashing : public Relation> { static constexpr size_t SR_PC_INCREMENTS_2 = 9; static constexpr size_t SR_ID_PROPAGATION = 10; static constexpr size_t SR_START_IS_SEPARATOR = 11; - static constexpr size_t SR_PADDING_CONSISTENCY = 14; - static constexpr size_t SR_PADDING_END = 15; - static constexpr size_t SR_PADDED_BY_ZERO_1 = 16; - static constexpr size_t SR_PADDED_BY_ZERO_2 = 17; - static constexpr size_t SR_PADDING_CORRECTNESS = 18; - static constexpr size_t SR_BYTECODE_LENGTH_FIELDS = 19; - static constexpr size_t SR_ROUNDS_DECREMENT = 20; - static constexpr size_t SR_HASH_IS_ID = 21; + static constexpr size_t SR_SEL_NOT_PADDING_1_REQUIRES_SEL = 14; + static constexpr size_t SR_SEL_NOT_PADDING_2_REQUIRES_SEL = 15; + static constexpr size_t SR_PADDING_CONSISTENCY = 16; + static constexpr size_t SR_PADDING_END = 17; + static constexpr size_t SR_PADDED_BY_ZERO_1 = 18; + static constexpr size_t SR_PADDED_BY_ZERO_2 = 19; + static constexpr size_t SR_PADDING_CORRECTNESS = 20; + static constexpr size_t SR_BYTECODE_LENGTH_FIELDS = 21; + static constexpr size_t SR_ROUNDS_DECREMENT = 22; + static constexpr size_t SR_HASH_IS_ID = 23; static std::string get_subrelation_label(size_t index) { @@ -72,6 +74,10 @@ template class bc_hashing : public Relation> { return "ID_PROPAGATION"; case SR_START_IS_SEPARATOR: return "START_IS_SEPARATOR"; + case SR_SEL_NOT_PADDING_1_REQUIRES_SEL: + return "SEL_NOT_PADDING_1_REQUIRES_SEL"; + case SR_SEL_NOT_PADDING_2_REQUIRES_SEL: + return "SEL_NOT_PADDING_2_REQUIRES_SEL"; case SR_PADDING_CONSISTENCY: return "PADDING_CONSISTENCY"; case SR_PADDING_END: diff --git a/barretenberg/cpp/src/barretenberg/vm2/generated/relations/bc_hashing_impl.hpp b/barretenberg/cpp/src/barretenberg/vm2/generated/relations/bc_hashing_impl.hpp index bb57bc9d65c2..2c6f24918361 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/generated/relations/bc_hashing_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/generated/relations/bc_hashing_impl.hpp @@ -116,28 +116,40 @@ void bc_hashingImpl::accumulate(ContainerOverSubrelations& evals, (FF(1) - static_cast(in.get(C::bc_hashing_sel_not_padding_2))); std::get<13>(evals) += (tmp * scaling_factor); } - { // PADDING_CONSISTENCY + { // SEL_NOT_PADDING_1_REQUIRES_SEL using View = typename std::tuple_element_t<14, ContainerOverSubrelations>::View; - auto tmp = CView(bc_hashing_PADDING_1) * static_cast(in.get(C::bc_hashing_sel_not_padding_2)); + auto tmp = static_cast(in.get(C::bc_hashing_sel_not_padding_1)) * + (FF(1) - static_cast(in.get(C::bc_hashing_sel))); std::get<14>(evals) += (tmp * scaling_factor); } - { // PADDING_END + { // SEL_NOT_PADDING_2_REQUIRES_SEL using View = typename std::tuple_element_t<15, ContainerOverSubrelations>::View; - auto tmp = CView(bc_hashing_PADDING_2) * (FF(1) - static_cast(in.get(C::bc_hashing_latch))); + auto tmp = static_cast(in.get(C::bc_hashing_sel_not_padding_2)) * + (FF(1) - static_cast(in.get(C::bc_hashing_sel))); std::get<15>(evals) += (tmp * scaling_factor); } - { // PADDED_BY_ZERO_1 + { // PADDING_CONSISTENCY using View = typename std::tuple_element_t<16, ContainerOverSubrelations>::View; - auto tmp = CView(bc_hashing_PADDING_1) * static_cast(in.get(C::bc_hashing_packed_fields_1)); + auto tmp = CView(bc_hashing_PADDING_1) * static_cast(in.get(C::bc_hashing_sel_not_padding_2)); std::get<16>(evals) += (tmp * scaling_factor); } - { // PADDED_BY_ZERO_2 + { // PADDING_END using View = typename std::tuple_element_t<17, ContainerOverSubrelations>::View; - auto tmp = CView(bc_hashing_PADDING_2) * static_cast(in.get(C::bc_hashing_packed_fields_2)); + auto tmp = CView(bc_hashing_PADDING_2) * (FF(1) - static_cast(in.get(C::bc_hashing_latch))); std::get<17>(evals) += (tmp * scaling_factor); } - { // PADDING_CORRECTNESS + { // PADDED_BY_ZERO_1 using View = typename std::tuple_element_t<18, ContainerOverSubrelations>::View; + auto tmp = CView(bc_hashing_PADDING_1) * static_cast(in.get(C::bc_hashing_packed_fields_1)); + std::get<18>(evals) += (tmp * scaling_factor); + } + { // PADDED_BY_ZERO_2 + using View = typename std::tuple_element_t<19, ContainerOverSubrelations>::View; + auto tmp = CView(bc_hashing_PADDING_2) * static_cast(in.get(C::bc_hashing_packed_fields_2)); + std::get<19>(evals) += (tmp * scaling_factor); + } + { // PADDING_CORRECTNESS + using View = typename std::tuple_element_t<20, ContainerOverSubrelations>::View; auto tmp = (static_cast(in.get(C::bc_hashing_pc_at_final_field)) - static_cast(in.get(C::bc_hashing_latch)) * (CView(bc_hashing_PADDING_1) * static_cast(in.get(C::bc_hashing_pc_index)) + @@ -145,17 +157,17 @@ void bc_hashingImpl::accumulate(ContainerOverSubrelations& evals, static_cast(in.get(C::bc_hashing_pc_index_1)) + static_cast(in.get(C::bc_hashing_sel_not_padding_2)) * static_cast(in.get(C::bc_hashing_pc_index_2)))); - std::get<18>(evals) += (tmp * scaling_factor); + std::get<20>(evals) += (tmp * scaling_factor); } { // BYTECODE_LENGTH_FIELDS - using View = typename std::tuple_element_t<19, ContainerOverSubrelations>::View; + using View = typename std::tuple_element_t<21, ContainerOverSubrelations>::View; auto tmp = static_cast(in.get(C::bc_hashing_latch)) * (FF(31) * (static_cast(in.get(C::bc_hashing_input_len)) - FF(2)) - static_cast(in.get(C::bc_hashing_pc_at_final_field))); - std::get<19>(evals) += (tmp * scaling_factor); + std::get<21>(evals) += (tmp * scaling_factor); } { // ROUNDS_DECREMENT - using View = typename std::tuple_element_t<20, ContainerOverSubrelations>::View; + using View = typename std::tuple_element_t<22, ContainerOverSubrelations>::View; auto tmp = static_cast(in.get(C::bc_hashing_sel)) * ((FF(1) - CView(bc_hashing_LATCH_CONDITION)) * ((static_cast(in.get(C::bc_hashing_rounds_rem_shift)) - @@ -163,14 +175,14 @@ void bc_hashingImpl::accumulate(ContainerOverSubrelations& evals, FF(1)) + static_cast(in.get(C::bc_hashing_latch)) * (static_cast(in.get(C::bc_hashing_rounds_rem)) - FF(1))); - std::get<20>(evals) += (tmp * scaling_factor); + std::get<22>(evals) += (tmp * scaling_factor); } { // HASH_IS_ID - using View = typename std::tuple_element_t<21, ContainerOverSubrelations>::View; + using View = typename std::tuple_element_t<23, ContainerOverSubrelations>::View; auto tmp = static_cast(in.get(C::bc_hashing_sel)) * (static_cast(in.get(C::bc_hashing_bytecode_id)) - static_cast(in.get(C::bc_hashing_output_hash))); - std::get<21>(evals) += (tmp * scaling_factor); + std::get<23>(evals) += (tmp * scaling_factor); } }