Skip to content

Commit 19a4e2a

Browse files
authored
feat: merge-train/barretenberg (#23269)
BEGIN_COMMIT_OVERRIDE fix(bb): push missing q_5 in QArith3Gate test to unbreak nightly debug build (#23229) fix: ecc/groups audit findings (#22864) chore: add tests around MSM invalid scalar handling (#23176) END_COMMIT_OVERRIDE
2 parents d8f7e77 + 4be7c4c commit 19a4e2a

13 files changed

Lines changed: 417 additions & 19 deletions

File tree

barretenberg/cpp/src/barretenberg/bbapi/bbapi_srs.cpp

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,15 @@
55
#include "barretenberg/bbapi/bbapi_srs.hpp"
66
#include "barretenberg/common/serialize.hpp"
77
#include "barretenberg/common/thread.hpp"
8+
#include "barretenberg/crypto/sha256/sha256.hpp"
89
#include "barretenberg/ecc/curves/bn254/g1.hpp"
910
#include "barretenberg/ecc/curves/bn254/g2.hpp"
1011
#include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp"
1112
#include "barretenberg/numeric/uint256/uint256.hpp"
13+
#include "barretenberg/srs/factories/bn254_crs_data.hpp"
14+
#include "barretenberg/srs/factories/bn254_g1_chunk_hashes.hpp"
1215
#include "barretenberg/srs/global_crs.hpp"
16+
#include <span>
1317

1418
namespace bb::bbapi {
1519

@@ -30,6 +34,23 @@ SrsInitSrs::Response SrsInitSrs::execute(BB_UNUSED BBApiRequest& request) &&
3034
}
3135
});
3236
} else if (bytes_per_point == COMPRESSED_POINT_SIZE) {
37+
// Verify SHA-256 of every 4 MB chunk against the in-binary pin BN254_G1_CHUNK_HASHES.
38+
// Require chunk-aligned input so every byte is covered (no partial trailing chunk).
39+
if (points_buf.size() == 0 || points_buf.size() % bb::srs::SRS_CHUNK_SIZE_BYTES != 0) {
40+
throw_or_abort("SrsInitSrs: compressed points_buf size " + std::to_string(points_buf.size()) +
41+
" must be a positive multiple of " + std::to_string(bb::srs::SRS_CHUNK_SIZE_BYTES));
42+
}
43+
size_t num_full_chunks = points_buf.size() / bb::srs::SRS_CHUNK_SIZE_BYTES;
44+
size_t chunks_to_verify = std::min(num_full_chunks, static_cast<size_t>(bb::srs::SRS_NUM_FULL_CHUNKS));
45+
for (size_t i = 0; i < chunks_to_verify; ++i) {
46+
auto chunk = std::span<const uint8_t>(points_buf.data() + i * bb::srs::SRS_CHUNK_SIZE_BYTES,
47+
bb::srs::SRS_CHUNK_SIZE_BYTES);
48+
auto hash = bb::crypto::sha256(chunk);
49+
if (hash != bb::srs::BN254_G1_CHUNK_HASHES[i]) {
50+
throw_or_abort("SrsInitSrs: g1 compressed chunk " + std::to_string(i) + " SHA-256 mismatch");
51+
}
52+
}
53+
3354
// Compressed: decompress and return uncompressed bytes for caller to cache
3455
parallel_for([&](ThreadChunk chunk) {
3556
for (auto i : chunk.range(static_cast<size_t>(num_points))) {
@@ -50,11 +71,23 @@ SrsInitSrs::Response SrsInitSrs::execute(BB_UNUSED BBApiRequest& request) &&
5071
std::to_string(bytes_per_point));
5172
}
5273

53-
// Parse G2 point from buffer (128 bytes). `serialize_from_buffer` validates that the bytes
54-
// decode to a curve point but does NOT enforce subgroup membership. BN254 G2 has a non-trivial
55-
// cofactor (h2 ≈ 2^254), so a curve point may lie in a small cofactor subgroup of order
56-
// dividing h2 rather than the prime-order subgroup of order r. Reject anything outside
57-
// the prime-order subgroup before it reaches the SRS factory.
74+
// Pin the first two G1 points to their canonical trusted-setup values. Defense in depth on the
75+
// compressed path; the only gate on the uncompressed (cached) path.
76+
if (num_points >= 1 && g1_points[0] != bb::srs::BN254_G1_FIRST_ELEMENT) {
77+
throw_or_abort("SrsInitSrs: g1_points[0] is not the canonical BN254 generator");
78+
}
79+
if (num_points >= 2 && g1_points[1] != bb::srs::get_bn254_g1_second_element()) {
80+
throw_or_abort("SrsInitSrs: g1_points[1] does not match the canonical trusted-setup tau·G");
81+
}
82+
83+
// Defense in depth: hash-pin AND subgroup-check the G2 input. Hash equality alone is sufficient
84+
// for the canonical case (it implies prime-order membership); the subgroup check is kept so
85+
// that any future relaxation of the hash gate (e.g. a flag to allow a different trusted setup)
86+
// does not silently reopen audit finding #7's small-subgroup attack.
87+
auto g2_hash = bb::crypto::sha256(std::span<const uint8_t>(g2_point.data(), g2_point.size()));
88+
if (g2_hash != bb::srs::BN254_G2_ELEMENT_SHA256) {
89+
throw_or_abort("SrsInitSrs: g2_point bytes do not match the canonical Aztec [x]_2 SHA-256");
90+
}
5891
auto g2_point_elem = from_buffer<g2::affine_element>(g2_point.data());
5992
if (!g2_point_elem.is_in_prime_subgroup()) {
6093
throw_or_abort("SrsInitSrs: g2_point is not in the BN254 G2 prime-order subgroup");

barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder_arithmetic.test.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,7 @@ TEST_F(UltraCircuitBuilderArithmetic, QArith3Gate)
549549
builder.blocks.arithmetic.q_2().emplace_back(q_2);
550550
builder.blocks.arithmetic.q_3().emplace_back(q_3);
551551
builder.blocks.arithmetic.q_4().emplace_back(q_4);
552+
builder.blocks.arithmetic.q_5().emplace_back(0);
552553
builder.blocks.arithmetic.q_c().emplace_back(q_c);
553554
builder.blocks.arithmetic.set_gate_selector(3);
554555
builder.check_selector_length_consistency();
@@ -561,6 +562,7 @@ TEST_F(UltraCircuitBuilderArithmetic, QArith3Gate)
561562
builder.blocks.arithmetic.q_2().emplace_back(0);
562563
builder.blocks.arithmetic.q_3().emplace_back(0);
563564
builder.blocks.arithmetic.q_4().emplace_back(0);
565+
builder.blocks.arithmetic.q_5().emplace_back(0);
564566
builder.blocks.arithmetic.q_c().emplace_back(0);
565567
builder.blocks.arithmetic.set_gate_selector(1);
566568
builder.check_selector_length_consistency();

barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_impl.hpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@ ecdsa_signature ecdsa_construct_signature(const std::string& message, const ecds
2929
Fr k = crypto::deterministic_nonce_rfc6979<Hash, Fr>(message, pkey_buffer);
3030
secure_erase(pkey_buffer);
3131

32-
// Compute R = k * G
33-
typename G1::affine_element R(G1::one * k);
32+
// Compute R = k * G. k is the secret RFC6979 nonce, so use the constant-time multiplication
33+
// to defend against the Hamming-weight / bit-length timing leak in operator*.
34+
typename G1::affine_element R(typename G1::element(G1::one).mul_const_time(k));
3435

3536
// Compute the signature
3637
Fr r = Fr(R.x);

barretenberg/cpp/src/barretenberg/crypto/schnorr/schnorr.tcc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,9 @@ schnorr_signature schnorr_construct_signature(const std::string& message, const
9090
//
9191
Fr k = Fr::random_element();
9292

93-
typename G1::affine_element R(G1::one * k);
93+
// k is a secret nonce; use the constant-time multiplication to defend against the
94+
// Hamming-weight / bit-length timing leak in operator*.
95+
typename G1::affine_element R(typename G1::element(G1::one).mul_const_time(k));
9496

9597
auto e_raw = schnorr_generate_challenge<Hash, G1>(message, public_key, R);
9698
// the conversion from e_raw results in a biased field element e

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

Lines changed: 137 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,9 @@ struct MsmScalar {
341341
}
342342
};
343343

344-
template <typename Builder> class MultiScalarMulInfinityTests : public ::testing::Test {
344+
// Shared single-term MSM circuit helpers: build a one point/one scalar MSM constraint with predicate=1
345+
// from explicit witness values, and run the resulting circuit.
346+
template <typename Builder> class MsmSingleTermFixture : public ::testing::Test {
345347
protected:
346348
static void SetUpTestSuite() { bb::srs::init_file_crs_factory(bb::srs::bb_crs_path()); }
347349

@@ -402,6 +404,8 @@ template <typename Builder> class MultiScalarMulInfinityTests : public ::testing
402404
}
403405
};
404406

407+
template <typename Builder> class MultiScalarMulInfinityTests : public MsmSingleTermFixture<Builder> {};
408+
405409
TYPED_TEST_SUITE(MultiScalarMulInfinityTests, BuilderTypes);
406410

407411
// scalar=0 → result = ∞: valid proof with out_point_is_infinite=1.
@@ -449,3 +453,135 @@ TYPED_TEST(MultiScalarMulInfinityTests, ForgedFiniteFlagOnInfinityResultFails)
449453
EXPECT_TRUE(!ok || err.find("assert_eq") != std::string::npos)
450454
<< "Forged finite flag on infinity result should fail";
451455
}
456+
457+
// ============================================================
458+
// Scalar field-bounds tests
459+
// ============================================================
460+
//
461+
// The MSM opcode receives a Grumpkin scalar as two field limbs: lo (low 128 bits) and hi (next 126
462+
// bits), reconstructing v = lo + hi * 2^128. cycle_scalar's public constructor adds an in-circuit
463+
// check that v < r, where r == bb::fq::modulus is the Grumpkin scalar field modulus (and also the
464+
// order of the Grumpkin group, since Grumpkin's scalar field is BN254's base field). batch_mul
465+
// additionally range-constrains the limbs to lo < 2^128 and hi < 2^126. These tests pin behaviour
466+
// at and beyond the modulus boundary: an out-of-range scalar must make the circuit unsatisfiable,
467+
// and the group law's s ≡ s + r equivalence must not let a caller smuggle a non-canonical scalar
468+
// through to barretenberg.
469+
470+
namespace {
471+
// r = order of the Grumpkin group = bb::fq::modulus.
472+
const uint256_t grumpkin_scalar_modulus = bb::fq::modulus;
473+
474+
// Build an MsmScalar straight from a uint256_t value, splitting at the 128-bit limb boundary with no
475+
// modular reduction (so out-of-field values can be expressed).
476+
MsmScalar msm_scalar_from_u256(const uint256_t& v)
477+
{
478+
return { MsmFF(v.slice(0, 128)), MsmFF(v.slice(128, 256)) };
479+
}
480+
} // namespace
481+
482+
template <typename Builder> class MultiScalarMulScalarBoundsTests : public MsmSingleTermFixture<Builder> {};
483+
484+
TYPED_TEST_SUITE(MultiScalarMulScalarBoundsTests, BuilderTypes);
485+
486+
// scalar == r: rejected. The in-circuit "scalar < r" check fails. (r·P = O, so the caller gains
487+
// nothing by claiming the point at infinity as the result.)
488+
TYPED_TEST(MultiScalarMulScalarBoundsTests, ScalarEqualToModulusFails)
489+
{
490+
BB_DISABLE_ASSERTS();
491+
MsmGrumpkinPoint point = MsmGrumpkinPoint::random_element();
492+
auto [constraint, witness] = TestFixture::make_msm(
493+
MsmAcirPoint::from_native(point), msm_scalar_from_u256(grumpkin_scalar_modulus), MsmAcirPoint::infinity());
494+
495+
auto [ok, err] = TestFixture::run_circuit(constraint, witness);
496+
EXPECT_FALSE(ok) << "scalar == Grumpkin scalar modulus must not produce a satisfiable circuit";
497+
}
498+
499+
// scalar == r + 1: rejected, even though (r + 1)·P == 1·P == P. The in-circuit "scalar < r" check
500+
// fails despite both limbs being within their range constraints.
501+
TYPED_TEST(MultiScalarMulScalarBoundsTests, ScalarModulusPlusOneFails)
502+
{
503+
BB_DISABLE_ASSERTS();
504+
MsmGrumpkinPoint point = MsmGrumpkinPoint::random_element();
505+
auto [constraint, witness] = TestFixture::make_msm(MsmAcirPoint::from_native(point),
506+
msm_scalar_from_u256(grumpkin_scalar_modulus + uint256_t(1)),
507+
MsmAcirPoint::from_native(point));
508+
509+
auto [ok, err] = TestFixture::run_circuit(constraint, witness);
510+
EXPECT_FALSE(ok) << "scalar == Grumpkin scalar modulus + 1 must not produce a satisfiable circuit";
511+
}
512+
513+
// scalar == r - 1: the largest in-field scalar. This must prove fine.
514+
TYPED_TEST(MultiScalarMulScalarBoundsTests, ScalarModulusMinusOneProves)
515+
{
516+
BB_DISABLE_ASSERTS();
517+
MsmGrumpkinPoint point = MsmGrumpkinPoint::random_element();
518+
bb::fq scalar_native = bb::fq(grumpkin_scalar_modulus - uint256_t(1));
519+
MsmGrumpkinPoint result = point * scalar_native;
520+
ASSERT_FALSE(result.is_point_at_infinity());
521+
auto [constraint, witness] = TestFixture::make_msm(MsmAcirPoint::from_native(point),
522+
msm_scalar_from_u256(grumpkin_scalar_modulus - uint256_t(1)),
523+
MsmAcirPoint::from_native(result));
524+
525+
auto [ok, err] = TestFixture::run_circuit(constraint, witness);
526+
EXPECT_TRUE(ok) << "scalar == Grumpkin scalar modulus - 1 (largest in-field scalar) should prove. err: " << err;
527+
}
528+
529+
// scalar == 2^254 - 1: the largest value the (128 + 126)-bit limb encoding can represent. Both limbs
530+
// satisfy their range constraints, so the only thing rejecting it is the in-circuit "scalar < r"
531+
// check — exercising the "limbs in range but value out of field" path.
532+
TYPED_TEST(MultiScalarMulScalarBoundsTests, MaxRepresentableScalarFails)
533+
{
534+
BB_DISABLE_ASSERTS();
535+
MsmGrumpkinPoint point = MsmGrumpkinPoint::random_element();
536+
uint256_t max_representable = (uint256_t(1) << 254) - uint256_t(1);
537+
MsmGrumpkinPoint result = point * bb::fq(max_representable); // (2^254 - 1) mod r
538+
auto [constraint, witness] = TestFixture::make_msm(
539+
MsmAcirPoint::from_native(point), msm_scalar_from_u256(max_representable), MsmAcirPoint::from_native(result));
540+
541+
auto [ok, err] = TestFixture::run_circuit(constraint, witness);
542+
EXPECT_FALSE(ok) << "scalar == 2^254 - 1 (> Grumpkin modulus) must not produce a satisfiable circuit";
543+
}
544+
545+
// hi limb == 2^126 (one bit too wide), lo == 0, i.e. scalar value 2^254. Both the limb range
546+
// constraint (hi < 2^126) and the "scalar < r" check reject it.
547+
TYPED_TEST(MultiScalarMulScalarBoundsTests, ScalarWithOversizedHiLimbFails)
548+
{
549+
BB_DISABLE_ASSERTS();
550+
MsmGrumpkinPoint point = MsmGrumpkinPoint::random_element();
551+
uint256_t two_pow_254 = uint256_t(1) << 254;
552+
MsmGrumpkinPoint result = point * bb::fq(two_pow_254);
553+
MsmScalar scalar{ MsmFF(0), MsmFF(uint256_t(1) << 126) }; // hi has 127 bits
554+
auto [constraint, witness] =
555+
TestFixture::make_msm(MsmAcirPoint::from_native(point), scalar, MsmAcirPoint::from_native(result));
556+
557+
auto [ok, err] = TestFixture::run_circuit(constraint, witness);
558+
EXPECT_FALSE(ok) << "scalar hi limb of 127 bits (value 2^254) must not produce a satisfiable circuit";
559+
}
560+
561+
// Group-law equivalence does not transfer: s·P == (s + r)·P, but the circuit accepts only the
562+
// canonical scalar s. Adding the Grumpkin modulus to a scalar cannot reprove the same output.
563+
TYPED_TEST(MultiScalarMulScalarBoundsTests, AddingGrumpkinModulusDoesNotReproveSameOutput)
564+
{
565+
BB_DISABLE_ASSERTS();
566+
MsmGrumpkinPoint point = MsmGrumpkinPoint::random_element();
567+
bb::fq scalar_native = bb::fq(5);
568+
MsmGrumpkinPoint result = point * scalar_native;
569+
ASSERT_FALSE(result.is_point_at_infinity());
570+
571+
// Sanity: the canonical scalar proves the result.
572+
{
573+
auto [constraint, witness] = TestFixture::make_msm(
574+
MsmAcirPoint::from_native(point), MsmScalar::from_native(scalar_native), MsmAcirPoint::from_native(result));
575+
auto [ok, err] = TestFixture::run_circuit(constraint, witness);
576+
EXPECT_TRUE(ok) << "canonical scalar should prove the MSM result. err: " << err;
577+
}
578+
579+
// The non-canonical scalar s + r yields the same point mathematically, but the circuit rejects it.
580+
{
581+
auto [constraint, witness] = TestFixture::make_msm(MsmAcirPoint::from_native(point),
582+
msm_scalar_from_u256(uint256_t(5) + grumpkin_scalar_modulus),
583+
MsmAcirPoint::from_native(result));
584+
auto [ok, err] = TestFixture::run_circuit(constraint, witness);
585+
EXPECT_FALSE(ok) << "scalar s + r must not reprove the output of scalar s";
586+
}
587+
}

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/pairing.test.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -268,10 +268,11 @@ TEST(pairing, PairingLinearityCheck)
268268
TEST(pairing, FinalExponentiation)
269269
{
270270
auto pow = [](const fq12& a, const fq& b) {
271+
const uint256_t exponent(b);
271272
fq12 result = fq12::one();
272273
fq12 base = a;
273274
for (size_t i = 0; i < 256; ++i) {
274-
if ((b.data[0] >> i) & 1) {
275+
if (exponent.get_bit(i)) {
275276
result *= base;
276277
}
277278
base = base.sqr();
@@ -291,10 +292,11 @@ TEST(pairing, FinalExponentiation)
291292
fq12 result = pairing::final_exponentiation_easy_part(element);
292293
result = pairing::final_exponentiation_tricky_part(result);
293294

294-
fq12 expected = element;
295-
expected = pairing::final_exponentiation_easy_part(expected);
296-
expected = pow(expected, mu0) + pow(expected, mu1).frobenius_map_one() + pow(expected, mu2).frobenius_map_two() +
295+
fq12 expected = pairing::final_exponentiation_easy_part(element);
296+
expected = pow(expected, mu0) * pow(expected, mu1).frobenius_map_one() * pow(expected, mu2).frobenius_map_two() *
297297
pow(expected, mu3).frobenius_map_three();
298+
299+
EXPECT_EQ(result, expected);
298300
}
299301

300302
TEST(pairing, Constants)

barretenberg/cpp/src/barretenberg/ecc/fields/field_impl.hpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -752,8 +752,13 @@ template <class T> constexpr uint64_t field<T>::is_msb_set_word() const noexcept
752752

753753
template <class T> constexpr bool field<T>::is_zero() const noexcept
754754
{
755-
return ((data[0] | data[1] | data[2] | data[3]) == 0) ||
756-
(data[0] == T::modulus_0 && data[1] == T::modulus_1 && data[2] == T::modulus_2 && data[3] == T::modulus_3);
755+
// Use bitwise OR (not || or && operator) so neither chain short-circuits: the running time must not depend on
756+
// whether the value is zero, on which limb of the modulus first matches/diverges, or on which form
757+
// (raw 0 vs the modulus) is being tested.
758+
const uint64_t raw_zero = data[0] | data[1] | data[2] | data[3];
759+
const uint64_t mod_zero =
760+
(data[0] ^ T::modulus_0) | (data[1] ^ T::modulus_1) | (data[2] ^ T::modulus_2) | (data[3] ^ T::modulus_3);
761+
return static_cast<bool>(static_cast<uint64_t>(raw_zero == 0) | static_cast<uint64_t>(mod_zero == 0));
757762
}
758763

759764
template <class T> constexpr field<T> field<T>::get_root_of_unity(size_t subgroup_size) noexcept

barretenberg/cpp/src/barretenberg/ecc/fields/prime_field.test.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,20 @@ TYPED_TEST(PrimeFieldTest, CompileTimeEquality)
117117
static_assert(!(a == f));
118118
}
119119

120+
TYPED_TEST(PrimeFieldTest, IsZeroOnModulusForm)
121+
{
122+
using F = TypeParam;
123+
124+
F modulus_form{ F::modulus.data[0], F::modulus.data[1], F::modulus.data[2], F::modulus.data[3] };
125+
EXPECT_TRUE(modulus_form.is_zero());
126+
127+
F prefix_match{ F::modulus.data[0], F::modulus.data[1], F::modulus.data[2], F::modulus.data[3] - 1 };
128+
EXPECT_FALSE(prefix_match.is_zero());
129+
130+
F first_limb_only{ F::modulus.data[0], 0, 0, 0 };
131+
EXPECT_FALSE(first_limb_only.is_zero());
132+
}
133+
120134
TYPED_TEST(PrimeFieldTest, CompileTimeSmallAddSubMul)
121135
{
122136
using F = TypeParam;

0 commit comments

Comments
 (0)