Skip to content

Commit 0842f45

Browse files
authored
feat: merge-train/barretenberg (#22447)
BEGIN_COMMIT_OVERRIDE fix: skip heavy recursion tests in debug builds (#22446) fix: add clear error for unsatisfiable ACIR AssertZero opcode (#22417) feat: enforce accumulator_not_empty = 0 at ECCVM lagrange_first row (#22461) fix: skip heavy recursion tests in debug builds, keep one for assertion coverage (#22389) fix: external audit fixes for Pedersen (#22434) chore!: fix BASE off-by-one in create_small_range_constraint in theta step of keccak (#22404) fix: external audit fixes for Keccak (#22436) fix: external audit fixes for BLAKE (#22443) chore: misc hash gadget updates (#22452) END_COMMIT_OVERRIDE
2 parents 0cccf64 + 94ab58d commit 0842f45

30 files changed

+546
-188
lines changed

barretenberg/cpp/bootstrap.sh

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,16 +245,21 @@ function test_cmds_native {
245245
awk '/^[a-zA-Z]/ {suite=$1} /^[ ]/ {print suite$1}' | \
246246
grep -v 'DISABLED_' | \
247247
while read -r test; do
248+
# Skip heavy recursion tests in debug builds — they take 400-600s and the same
249+
# code paths are already exercised (with assertions) by faster tests in the suite.
250+
# Keep WithoutPredicate/1.GenerateVKFromConstraints (241s) so that the debug-only
251+
# native_verification_debug path in honk_recursion_constraint.cpp is still exercised.
252+
if [[ "$native_preset" == *debug* ]] && [[ "$test" =~ ^(HonkRecursionConstraintTest|ChonkRecursionConstraintTest|AvmRecursionInnerCircuitTests) ]]; then
253+
if [[ "$test" != "HonkRecursionConstraintTestWithoutPredicate/1.GenerateVKFromConstraints" ]]; then
254+
continue
255+
fi
256+
fi
248257
local prefix=$hash
249258
# A little extra resource for these tests.
250259
# IPARecursiveTests fails with 2 threads.
251260
if [[ "$test" =~ ^(AcirAvmRecursionConstraint|ChonkKernelCapacity|AvmRecursiveTests|IPARecursiveTests|HonkRecursionConstraintTest|ChonkRecursionConstraintTest) ]]; then
252261
prefix="$prefix:CPUS=4:MEM=8g"
253262
fi
254-
# These tests routinely take 400-600s in debug builds; bump from the 600s default.
255-
if [[ "$test" =~ ^(HonkRecursionConstraintTest|ChonkRecursionConstraintTest) ]]; then
256-
prefix="$prefix:TIMEOUT=900s"
257-
fi
258263
echo -e "$prefix barretenberg/cpp/scripts/run_test.sh $bin_name $test"
259264
done || (echo "Failed to list tests in $bin" && exit 1)
260265
done

barretenberg/cpp/scripts/test_chonk_standalone_vks_havent_changed.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ script_path="$root/barretenberg/cpp/scripts/test_chonk_standalone_vks_havent_cha
2121
# - Generate a hash for versioning: sha256sum bb-chonk-inputs.tar.gz
2222
# - Upload the compressed results: aws s3 cp bb-chonk-inputs.tar.gz s3://aztec-ci-artifacts/protocol/bb-chonk-inputs-[hash(0:8)].tar.gz
2323
# Note: In case of the "Test suite failed to run ... Unexpected token 'with' " error, need to run: docker pull aztecprotocol/build:3.0
24-
pinned_short_hash="50947760"
24+
pinned_short_hash="d519f639"
2525
pinned_chonk_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-chonk-inputs-${pinned_short_hash}.tar.gz"
2626

2727
function update_pinned_hash_in_script {
@@ -66,7 +66,7 @@ function check_circuit_vks {
6666
bb_check_args+=(--disable_asserts)
6767
fi
6868

69-
output=$($bb "${bb_check_args[@]}") || exit_code=$?
69+
output=$($bb "${bb_check_args[@]}" 2>&1) || exit_code=$?
7070

7171
if [[ $exit_code -ne 0 ]]; then
7272
# Check if this is actually a VK change

barretenberg/cpp/src/barretenberg/crypto/blake3s/blake3s.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ enum blake3s_constant {
6161
BLAKE3_MAX_DEPTH = 54
6262
};
6363

64-
using key_array = std::array<uint32_t, BLAKE3_KEY_LEN>;
64+
using key_array = std::array<uint32_t, BLAKE3_KEY_LEN / sizeof(uint32_t)>;
6565
using block_array = std::array<uint8_t, BLAKE3_BLOCK_LEN>;
6666
using state_array = std::array<uint32_t, 16>;
6767
using out_array = std::array<uint8_t, BLAKE3_OUT_LEN>;

barretenberg/cpp/src/barretenberg/crypto/blake3s/blake3s.tcc

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ struct output_t {
180180
constexpr output_t make_output(const key_array& input_cv, const uint8_t* block, uint8_t block_len, uint8_t flags)
181181
{
182182
output_t ret;
183-
for (size_t i = 0; i < (BLAKE3_OUT_LEN >> 2); ++i) {
183+
for (size_t i = 0; i < ret.input_cv.size(); ++i) {
184184
ret.input_cv[i] = input_cv[i];
185185
}
186186
for (size_t i = 0; i < BLAKE3_BLOCK_LEN; i++) {
@@ -193,7 +193,7 @@ constexpr output_t make_output(const key_array& input_cv, const uint8_t* block,
193193

194194
constexpr void blake3_hasher_init(blake3_hasher* self)
195195
{
196-
for (size_t i = 0; i < (BLAKE3_KEY_LEN >> 2); ++i) {
196+
for (size_t i = 0; i < IV.size(); ++i) {
197197
self->key[i] = IV[i];
198198
self->cv[i] = IV[i];
199199
}
@@ -260,6 +260,11 @@ std::vector<uint8_t> blake3s(std::vector<uint8_t> const& input)
260260

261261
constexpr std::array<uint8_t, BLAKE3_OUT_LEN> blake3s_constexpr(const uint8_t* input, const size_t input_size)
262262
{
263+
if (std::is_constant_evaluated()) {
264+
if (input_size > 1024U) {
265+
__builtin_trap();
266+
}
267+
}
263268
blake3_hasher hasher;
264269
blake3_hasher_init(&hasher);
265270
blake3_hasher_update(&hasher, input, input_size);

barretenberg/cpp/src/barretenberg/crypto/keccak/keccak.cpp

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -110,30 +110,3 @@ struct keccak256 ethash_keccak256(const uint8_t* data, size_t size) NOEXCEPT
110110
keccak(hash.word64s, 256, data, size);
111111
return hash;
112112
}
113-
114-
struct keccak256 hash_field_elements(const uint64_t* limbs, size_t num_elements)
115-
{
116-
std::vector<uint8_t> input_buffer(num_elements * KECCAK256_OUTPUT_BYTES);
117-
118-
for (size_t i = 0; i < num_elements; ++i) {
119-
for (size_t j = 0; j < KECCAK256_OUTPUT_WORDS; ++j) {
120-
uint64_t word = (limbs[i * KECCAK256_OUTPUT_WORDS + j]);
121-
size_t idx = i * 32 + j * 8;
122-
input_buffer[idx] = (uint8_t)((word >> 56) & 0xff);
123-
input_buffer[idx + 1] = (uint8_t)((word >> 48) & 0xff);
124-
input_buffer[idx + 2] = (uint8_t)((word >> 40) & 0xff);
125-
input_buffer[idx + 3] = (uint8_t)((word >> 32) & 0xff);
126-
input_buffer[idx + 4] = (uint8_t)((word >> 24) & 0xff);
127-
input_buffer[idx + 5] = (uint8_t)((word >> 16) & 0xff);
128-
input_buffer[idx + 6] = (uint8_t)((word >> 8) & 0xff);
129-
input_buffer[idx + 7] = (uint8_t)(word & 0xff);
130-
}
131-
}
132-
133-
return ethash_keccak256(input_buffer.data(), num_elements * KECCAK256_OUTPUT_BYTES);
134-
}
135-
136-
struct keccak256 hash_field_element(const uint64_t* limb)
137-
{
138-
return hash_field_elements(limb, 1);
139-
}

barretenberg/cpp/src/barretenberg/crypto/keccak/keccak.hpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,6 @@ void ethash_keccakf1600(uint64_t state[KECCAKF1600_LANES]) NOEXCEPT;
4040

4141
struct keccak256 ethash_keccak256(const uint8_t* data, size_t size) NOEXCEPT;
4242

43-
struct keccak256 hash_field_elements(const uint64_t* limbs, size_t num_elements);
44-
45-
struct keccak256 hash_field_element(const uint64_t* limb);
46-
4743
namespace bb::crypto {
4844
/**
4945
* @brief A wrapper class used to construct `KeccakTranscript`.

barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ std::vector<typename Curve::BaseField> pedersen_hash_base<Curve>::convert_buffer
7777
template <typename Curve>
7878
typename Curve::BaseField pedersen_hash_base<Curve>::hash(const std::vector<Fq>& inputs, const GeneratorContext context)
7979
{
80+
if (inputs.empty()) {
81+
throw_or_abort("pedersen hash: empty input");
82+
}
83+
8084
Element result = length_generator * Fr(inputs.size());
8185
return (result + pedersen_commitment_base<Curve>::commit_native(inputs, context)).normalize().x;
8286
}
@@ -88,6 +92,10 @@ template <typename Curve>
8892
typename Curve::BaseField pedersen_hash_base<Curve>::hash_buffer(const std::vector<uint8_t>& input,
8993
const GeneratorContext context)
9094
{
95+
if (input.empty()) {
96+
throw_or_abort("pedersen hash_buffer: empty input");
97+
}
98+
9199
std::vector<Fq> converted = convert_buffer(input);
92100

93101
if (converted.size() < 2) {

barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,5 +64,26 @@ TEST(Pedersen, Hash32Bytes)
6464

6565
EXPECT_EQ(got, expected);
6666
}
67+
// Verifies that hashing an empty input throws an exception
68+
TEST(Pedersen, HashRejectsEmptyInput)
69+
{
70+
EXPECT_THROW(
71+
{
72+
auto result = pedersen_hash::hash({});
73+
static_cast<void>(result);
74+
},
75+
std::runtime_error);
76+
}
77+
78+
// Verifies that hashing an empty input throws an exception
79+
TEST(Pedersen, HashBufferRejectsEmptyInput)
80+
{
81+
EXPECT_THROW(
82+
{
83+
auto result = pedersen_hash::hash_buffer({});
84+
static_cast<void>(result);
85+
},
86+
std::runtime_error);
87+
}
6788

6889
} // namespace bb::crypto

barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,7 @@ TYPED_TEST(AcirFormatTests, ExpressionWithOnlyConstantTermFails)
3636
.return_values = {},
3737
};
3838

39-
EXPECT_THROW_WITH_MESSAGE(circuit_serde_to_acir_format(circuit),
40-
"split_into_mul_quad_gates: resulted in zero gates.");
39+
EXPECT_THROW_WITH_MESSAGE(circuit_serde_to_acir_format(circuit), "circuit is unsatisfiable");
4140
}
4241

4342
TYPED_TEST(AcirFormatTests, ExpressionWithCancellingCoefficientsFails)
Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
/**
2+
* @file acir_null_deref.test.cpp
3+
* @brief Exploit and fix tests for null shared_ptr dereference in ACIR deserialization.
4+
*
5+
* Demonstrates that crafted ACIR bytecode containing msgpack NIL values for
6+
* shared_ptr<array<T,N>> fields would produce a null pointer dereference in
7+
* acir_to_constraint_buf.cpp, and that the fix (rejecting NIL in conv_fld_from_kvmap
8+
* and conv_fld_from_array) prevents the crash.
9+
*
10+
* Attack vector: An attacker crafts raw ACIR bytecode (bypassing the Noir compiler)
11+
* containing a BlackBoxFuncCall opcode where a fixed-size array field is encoded as
12+
* msgpack NIL (0xc0). Without the fix, the AztecProtocol/msgpack-c fork silently
13+
* converts NIL to a null shared_ptr, which is then dereferenced unconditionally.
14+
* With the fix, deserialization rejects NIL for required fields and throws.
15+
*/
16+
17+
#include <gtest/gtest.h>
18+
#include <memory>
19+
#include <regex>
20+
#include <vector>
21+
22+
#include "acir_to_constraint_buf.hpp"
23+
#include "barretenberg/common/assert.hpp"
24+
#include "barretenberg/serialize/msgpack_impl.hpp"
25+
#include "serde/acir.hpp"
26+
27+
using namespace acir_format;
28+
29+
class AcirNullDerefTest : public ::testing::Test {};
30+
31+
/**
32+
* Helper: build a ProgramWithoutBrillig wire format by manually packing with msgpack.
33+
* We can't use ProgramWithoutBrillig::msgpack_pack because std::monostate lacks a pack adaptor.
34+
* Instead, we pack the structure by hand: ARRAY[ARRAY[circuit], NIL].
35+
*/
36+
static std::vector<uint8_t> serialize_program_with_circuit(const Acir::Circuit& circuit)
37+
{
38+
msgpack::sbuffer sbuf;
39+
msgpack::packer<msgpack::sbuffer> packer(sbuf);
40+
41+
// ProgramWithoutBrillig = ARRAY[functions, unconstrained_functions]
42+
packer.pack_array(2);
43+
44+
// functions = ARRAY[circuit]
45+
packer.pack_array(1);
46+
packer.pack(circuit);
47+
48+
// unconstrained_functions = NIL (std::monostate — ignored by ProgramWithoutBrillig::msgpack_unpack)
49+
packer.pack_nil();
50+
51+
// Prepend format marker 0x03 (FORMAT_MSGPACK_COMPACT)
52+
std::vector<uint8_t> buf;
53+
buf.reserve(1 + sbuf.size());
54+
buf.push_back(0x03);
55+
buf.insert(buf.end(), sbuf.data(), sbuf.data() + sbuf.size());
56+
return buf;
57+
}
58+
59+
// ============================================================================
60+
// Exploit tests: These construct null shared_ptr directly in the struct,
61+
// bypassing deserialization. They prove the dereference sites are unguarded.
62+
// ============================================================================
63+
64+
TEST_F(AcirNullDerefTest, AES128Encrypt_NullIV_DirectCircuit_Crashes)
65+
{
66+
Acir::BlackBoxFuncCall::AES128Encrypt aes_op{
67+
.inputs = {},
68+
.iv = nullptr,
69+
.key = nullptr,
70+
.outputs = {},
71+
};
72+
73+
Acir::BlackBoxFuncCall bbfc;
74+
bbfc.value = aes_op;
75+
76+
Acir::Circuit circuit{
77+
.opcodes = { Acir::Opcode{ Acir::Opcode::BlackBoxFuncCall{ .value = bbfc } } },
78+
.public_parameters = {},
79+
.return_values = {},
80+
};
81+
82+
// Dereferences nullptr at acir_to_constraint_buf.cpp:636: *arg.iv
83+
EXPECT_DEATH(circuit_serde_to_acir_format(circuit), "");
84+
}
85+
86+
TEST_F(AcirNullDerefTest, Keccakf1600_NullInputs_DirectCircuit_Crashes)
87+
{
88+
Acir::BlackBoxFuncCall::Keccakf1600 keccak_op{
89+
.inputs = nullptr,
90+
.outputs = nullptr,
91+
};
92+
93+
Acir::BlackBoxFuncCall bbfc;
94+
bbfc.value = keccak_op;
95+
96+
Acir::Circuit circuit{
97+
.opcodes = { Acir::Opcode{ Acir::Opcode::BlackBoxFuncCall{ .value = bbfc } } },
98+
.public_parameters = {},
99+
.return_values = {},
100+
};
101+
102+
// Dereferences nullptr at acir_to_constraint_buf.cpp:716: *arg.inputs
103+
EXPECT_DEATH(circuit_serde_to_acir_format(circuit), "");
104+
}
105+
106+
TEST_F(AcirNullDerefTest, Sha256Compression_NullInputs_DirectCircuit_Crashes)
107+
{
108+
Acir::BlackBoxFuncCall::Sha256Compression sha_op{
109+
.inputs = nullptr,
110+
.hash_values = nullptr,
111+
.outputs = nullptr,
112+
};
113+
114+
Acir::BlackBoxFuncCall bbfc;
115+
bbfc.value = sha_op;
116+
117+
Acir::Circuit circuit{
118+
.opcodes = { Acir::Opcode{ Acir::Opcode::BlackBoxFuncCall{ .value = bbfc } } },
119+
.public_parameters = {},
120+
.return_values = {},
121+
};
122+
123+
// Dereferences nullptr at acir_to_constraint_buf.cpp:644: *arg.inputs
124+
EXPECT_DEATH(circuit_serde_to_acir_format(circuit), "");
125+
}
126+
127+
// ============================================================================
128+
// Fix tests: These go through the deserialization path (raw bytes or msgpack
129+
// roundtrip). The fix in conv_fld_from_array rejects NIL before it can become
130+
// a null shared_ptr, throwing instead of crashing.
131+
// ============================================================================
132+
133+
TEST_F(AcirNullDerefTest, AES128Encrypt_NullIV_FromBytes_ThrowsAfterFix)
134+
{
135+
Acir::BlackBoxFuncCall::AES128Encrypt aes_op{
136+
.inputs = {},
137+
.iv = nullptr, // Serialized as NIL (0xc0)
138+
.key = nullptr,
139+
.outputs = {},
140+
};
141+
142+
Acir::BlackBoxFuncCall bbfc;
143+
bbfc.value = aes_op;
144+
145+
Acir::Circuit circuit{
146+
.opcodes = { Acir::Opcode{ Acir::Opcode::BlackBoxFuncCall{ .value = bbfc } } },
147+
.public_parameters = {},
148+
.return_values = {},
149+
};
150+
151+
auto buf = serialize_program_with_circuit(circuit);
152+
153+
// Verify the buffer contains NIL byte (0xc0) from the null shared_ptr
154+
size_t nil_count = 0;
155+
for (uint8_t b : buf) {
156+
if (b == 0xc0) {
157+
nil_count++;
158+
}
159+
}
160+
ASSERT_GE(nil_count, 2U) << "Buffer must contain at least 2 NIL (0xc0) bytes for null iv and key";
161+
162+
// After fix: deserialization rejects NIL for required fields → throws instead of SIGSEGV
163+
EXPECT_THROW_WITH_MESSAGE(circuit_buf_to_acir_format(std::move(buf)), "nil value for required field");
164+
}
165+
166+
TEST_F(AcirNullDerefTest, NullSharedPtr_RejectedByMsgpackRoundtrip)
167+
{
168+
// After the fix, a null shared_ptr encoded as NIL is rejected during deserialization.
169+
// conv_fld_from_array detects NIL at the array slot and throws before converting.
170+
Acir::BlackBoxFuncCall::AES128Encrypt original{
171+
.inputs = {},
172+
.iv = nullptr,
173+
.key = nullptr,
174+
.outputs = {},
175+
};
176+
177+
// Serialize (null shared_ptr → NIL byte 0xc0)
178+
msgpack::sbuffer sbuf;
179+
msgpack::pack(sbuf, original);
180+
181+
// Deserialize — should throw because iv slot is NIL
182+
auto oh = msgpack::unpack(sbuf.data(), sbuf.size());
183+
Acir::BlackBoxFuncCall::AES128Encrypt deserialized;
184+
EXPECT_THROW_WITH_MESSAGE(oh.get().convert(deserialized), "nil value for required field");
185+
}

0 commit comments

Comments
 (0)