Skip to content

Commit 97e3802

Browse files
authored
chore: UB updates (#22308)
Fixes the following from UB meta [issue](AztecProtocol/barretenberg-claude#2414): - F11 (zk_sumcheck_data.hpp) — Signed 1 << n is UB when n≥31; changed to 1UL << - F10 (rom_table.cpp, ram_table.cpp, twin_rom_table.cpp, databus.cpp) — OOB vector access after context->failure() soft fail; added early returns - F9 (field_declarations.hpp, field_impl_x64.hpp) — asm_self_* functions write through const& which is UB; changed to mutable ref
1 parent 4373838 commit 97e3802

File tree

6 files changed

+35
-4
lines changed

6 files changed

+35
-4
lines changed

barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ field_t<Builder> databus<Builder>::bus_vector::operator[](const field_pt& index)
4545
// Ensure the read is valid
4646
auto raw_index = static_cast<size_t>(uint256_t(index.get_value()).data[0]);
4747
if (raw_index >= length) {
48+
// Set a failure when the index is out of bounds. Return early to avoid OOB vector access.
4849
context->failure("bus_vector: access out of bounds");
50+
return field_pt::from_witness_index(context, context->zero_idx());
4951
}
5052

5153
// The read index must be a witness; if constant, add it as a constant variable

barretenberg/cpp/src/barretenberg/stdlib/primitives/memory/ram_table.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,9 @@ template <typename Builder> field_t<Builder> ram_table<Builder>::read(const fiel
180180

181181
const auto native_index = uint256_t(index.get_value());
182182
if (native_index >= length) {
183-
// set a failure when the index is out of bounds. another error will be thrown when we try to call
184-
// `read_RAM_array`.
183+
// Set a failure when the index is out of bounds. Return early to avoid OOB vector access.
185184
context->failure("ram_table: RAM array access out of bounds");
185+
return field_pt::from_witness_index(context, context->zero_idx());
186186
}
187187

188188
if (!check_indices_initialized()) {

barretenberg/cpp/src/barretenberg/stdlib/primitives/memory/rom_table.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,9 @@ template <typename Builder> field_t<Builder> rom_table<Builder>::operator[](cons
178178

179179
const auto native_index = uint256_t(index.get_value());
180180
if (native_index >= length) {
181+
// Set a failure when the index is out of bounds. Return early to avoid OOB vector access.
181182
context->failure("rom_table: ROM array access out of bounds");
183+
return field_pt::from_witness_index(context, context->zero_idx());
182184
}
183185

184186
uint32_t output_idx = context->read_ROM_array(rom_id, index.get_witness_index());
@@ -188,7 +190,6 @@ template <typename Builder> field_t<Builder> rom_table<Builder>::operator[](cons
188190

189191
// If the index is legitimate, restore the tag
190192
if (native_index < length) {
191-
192193
element.set_origin_tag(_tags[cast_index]);
193194
}
194195
return element;

barretenberg/cpp/src/barretenberg/stdlib/primitives/memory/rom_table.test.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,3 +167,28 @@ TEST(RomTable, OobConstantIndexDoesNotCrashRegression)
167167

168168
EXPECT_TRUE(builder.failed());
169169
}
170+
171+
/**
172+
* @brief Regression: OOB witness-index read must soft-fail without OOB vector access.
173+
*/
174+
TEST(RomTable, OobWitnessIndexDoesNotCrashRegression)
175+
{
176+
using Builder = UltraCircuitBuilder;
177+
using field_ct = stdlib::field_t<Builder>;
178+
using witness_ct = stdlib::witness_t<Builder>;
179+
using rom_table_ct = stdlib::rom_table<Builder>;
180+
181+
Builder builder;
182+
183+
std::vector<field_ct> table_values;
184+
table_values.emplace_back(witness_ct(&builder, bb::fr(1)));
185+
table_values.emplace_back(witness_ct(&builder, bb::fr(2)));
186+
table_values.emplace_back(witness_ct(&builder, bb::fr(3)));
187+
rom_table_ct table(table_values);
188+
189+
// OOB witness index — should soft-fail, not crash
190+
field_ct oob_index = witness_ct(&builder, bb::fr(100000000));
191+
table[oob_index];
192+
193+
EXPECT_TRUE(builder.failed());
194+
}

barretenberg/cpp/src/barretenberg/stdlib/primitives/memory/twin_rom_table.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,10 @@ std::array<field_t<Builder>, 2> twin_rom_table<Builder>::operator[](const field_
150150

151151
initialize_table();
152152
if (uint256_t(index.get_value()) >= length) {
153+
// Set a failure when the index is out of bounds. Return early to avoid OOB vector access.
153154
context->failure("twin_rom_table: ROM array access out of bounds");
155+
return { field_pt::from_witness_index(context, context->zero_idx()),
156+
field_pt::from_witness_index(context, context->zero_idx()) };
154157
}
155158

156159
auto output_indices = context->read_ROM_array_pair(rom_id, index.get_witness_index());

barretenberg/cpp/src/barretenberg/sumcheck/zk_sumcheck_data.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ template <typename Flavor> struct ZKSumcheckData {
151151
}
152152
total_sum *= scaling_factor;
153153

154-
return { total_sum + constant_term * (static_cast<uint64_t>(1) << libra_univariates.size()), scaling_factor };
154+
return { total_sum + constant_term * (1UL << libra_univariates.size()), scaling_factor };
155155
}
156156

157157
/**

0 commit comments

Comments
 (0)