fix: ecc/groups audit findings

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

@@ -5,11 +5,15 @@
 #include "barretenberg/bbapi/bbapi_srs.hpp"
 #include "barretenberg/common/serialize.hpp"
 #include "barretenberg/common/thread.hpp"
+#include "barretenberg/crypto/sha256/sha256.hpp"
 #include "barretenberg/ecc/curves/bn254/g1.hpp"
 #include "barretenberg/ecc/curves/bn254/g2.hpp"
 #include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp"
 #include "barretenberg/numeric/uint256/uint256.hpp"
+#include "barretenberg/srs/factories/bn254_crs_data.hpp"
+#include "barretenberg/srs/factories/bn254_g1_chunk_hashes.hpp"
 #include "barretenberg/srs/global_crs.hpp"
+#include <span>
 
 namespace bb::bbapi {
 
@@ -30,6 +34,23 @@ SrsInitSrs::Response SrsInitSrs::execute(BB_UNUSED BBApiRequest& request) &&
             }
         });
     } else if (bytes_per_point == COMPRESSED_POINT_SIZE) {
+        // Verify SHA-256 of every 4 MB chunk against the in-binary pin BN254_G1_CHUNK_HASHES.
+        // Require chunk-aligned input so every byte is covered (no partial trailing chunk).
+        if (points_buf.size() == 0 || points_buf.size() % bb::srs::SRS_CHUNK_SIZE_BYTES != 0) {
+            throw_or_abort("SrsInitSrs: compressed points_buf size " + std::to_string(points_buf.size()) +
+                           " must be a positive multiple of " + std::to_string(bb::srs::SRS_CHUNK_SIZE_BYTES));
+        }
+        size_t num_full_chunks = points_buf.size() / bb::srs::SRS_CHUNK_SIZE_BYTES;
+        size_t chunks_to_verify = std::min(num_full_chunks, static_cast<size_t>(bb::srs::SRS_NUM_FULL_CHUNKS));
+        for (size_t i = 0; i < chunks_to_verify; ++i) {
+            auto chunk = std::span<const uint8_t>(points_buf.data() + i * bb::srs::SRS_CHUNK_SIZE_BYTES,
+                                                  bb::srs::SRS_CHUNK_SIZE_BYTES);
+            auto hash = bb::crypto::sha256(chunk);
+            if (hash != bb::srs::BN254_G1_CHUNK_HASHES[i]) {
+                throw_or_abort("SrsInitSrs: g1 compressed chunk " + std::to_string(i) + " SHA-256 mismatch");
+            }
+        }
+
         // Compressed: decompress and return uncompressed bytes for caller to cache
         parallel_for([&](ThreadChunk chunk) {
             for (auto i : chunk.range(static_cast<size_t>(num_points))) {
@@ -50,11 +71,23 @@ SrsInitSrs::Response SrsInitSrs::execute(BB_UNUSED BBApiRequest& request) &&
                        std::to_string(bytes_per_point));
     }
 
-    // Parse G2 point from buffer (128 bytes). `serialize_from_buffer` validates that the bytes
-    // decode to a curve point but does NOT enforce subgroup membership. BN254 G2 has a non-trivial
-    // cofactor (h2 ≈ 2^254), so a curve point may lie in a small cofactor subgroup of order
-    // dividing h2 rather than the prime-order subgroup of order r. Reject anything outside
-    // the prime-order subgroup before it reaches the SRS factory.
+    // Pin the first two G1 points to their canonical trusted-setup values. Defense in depth on the
+    // compressed path; the only gate on the uncompressed (cached) path.
+    if (num_points >= 1 && g1_points[0] != bb::srs::BN254_G1_FIRST_ELEMENT) {
+        throw_or_abort("SrsInitSrs: g1_points[0] is not the canonical BN254 generator");
+    }
+    if (num_points >= 2 && g1_points[1] != bb::srs::get_bn254_g1_second_element()) {
+        throw_or_abort("SrsInitSrs: g1_points[1] does not match the canonical trusted-setup tau·G");
+    }
+
+    // Defense in depth: hash-pin AND subgroup-check the G2 input. Hash equality alone is sufficient
+    // for the canonical case (it implies prime-order membership); the subgroup check is kept so
+    // that any future relaxation of the hash gate (e.g. a flag to allow a different trusted setup)
+    // does not silently reopen audit finding #7's small-subgroup attack.
+    auto g2_hash = bb::crypto::sha256(std::span<const uint8_t>(g2_point.data(), g2_point.size()));
+    if (g2_hash != bb::srs::BN254_G2_ELEMENT_SHA256) {
+        throw_or_abort("SrsInitSrs: g2_point bytes do not match the canonical Aztec [x]_2 SHA-256");
+    }
     auto g2_point_elem = from_buffer<g2::affine_element>(g2_point.data());
     if (!g2_point_elem.is_in_prime_subgroup()) {
         throw_or_abort("SrsInitSrs: g2_point is not in the BN254 G2 prime-order subgroup");

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

@@ -29,8 +29,9 @@ ecdsa_signature ecdsa_construct_signature(const std::string& message, const ecds
     Fr k = crypto::deterministic_nonce_rfc6979<Hash, Fr>(message, pkey_buffer);
     secure_erase(pkey_buffer);
 
-    // Compute R = k * G
-    typename G1::affine_element R(G1::one * k);
+    // Compute R = k * G. k is the secret RFC6979 nonce, so use the constant-time multiplication
+    // to defend against the Hamming-weight / bit-length timing leak in operator*.
+    typename G1::affine_element R(typename G1::element(G1::one).mul_const_time(k));
 
     // Compute the signature
     Fr r = Fr(R.x);

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

@@ -90,7 +90,9 @@ schnorr_signature schnorr_construct_signature(const std::string& message, const
     //
     Fr k = Fr::random_element();
 
-    typename G1::affine_element R(G1::one * k);
+    // k is a secret nonce; use the constant-time multiplication to defend against the
+    // Hamming-weight / bit-length timing leak in operator*.
+    typename G1::affine_element R(typename G1::element(G1::one).mul_const_time(k));
 
     auto e_raw = schnorr_generate_challenge<Hash, G1>(message, public_key, R);
     // the conversion from e_raw results in a biased field element e

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

@@ -268,10 +268,11 @@ TEST(pairing, PairingLinearityCheck)
 TEST(pairing, FinalExponentiation)
 {
     auto pow = [](const fq12& a, const fq& b) {
+        const uint256_t exponent(b);
         fq12 result = fq12::one();
         fq12 base = a;
         for (size_t i = 0; i < 256; ++i) {
-            if ((b.data[0] >> i) & 1) {
+            if (exponent.get_bit(i)) {
                 result *= base;
             }
             base = base.sqr();
@@ -291,10 +292,11 @@ TEST(pairing, FinalExponentiation)
     fq12 result = pairing::final_exponentiation_easy_part(element);
     result = pairing::final_exponentiation_tricky_part(result);
 
-    fq12 expected = element;
-    expected = pairing::final_exponentiation_easy_part(expected);
-    expected = pow(expected, mu0) + pow(expected, mu1).frobenius_map_one() + pow(expected, mu2).frobenius_map_two() +
+    fq12 expected = pairing::final_exponentiation_easy_part(element);
+    expected = pow(expected, mu0) * pow(expected, mu1).frobenius_map_one() * pow(expected, mu2).frobenius_map_two() *
                pow(expected, mu3).frobenius_map_three();
+
+    EXPECT_EQ(result, expected);
 }
 
 TEST(pairing, Constants)

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

@@ -752,8 +752,13 @@ template <class T> constexpr uint64_t field<T>::is_msb_set_word() const noexcept
 
 template <class T> constexpr bool field<T>::is_zero() const noexcept
 {
-    return ((data[0] | data[1] | data[2] | data[3]) == 0) ||
-           (data[0] == T::modulus_0 && data[1] == T::modulus_1 && data[2] == T::modulus_2 && data[3] == T::modulus_3);
+    // Use bitwise OR (not || or && operator) so neither chain short-circuits: the running time must not depend on
+    // whether the value is zero, on which limb of the modulus first matches/diverges, or on which form
+    // (raw 0 vs the modulus) is being tested.
+    const uint64_t raw_zero = data[0] | data[1] | data[2] | data[3];
+    const uint64_t mod_zero =
+        (data[0] ^ T::modulus_0) | (data[1] ^ T::modulus_1) | (data[2] ^ T::modulus_2) | (data[3] ^ T::modulus_3);
+    return static_cast<bool>(static_cast<uint64_t>(raw_zero == 0) | static_cast<uint64_t>(mod_zero == 0));
 }
 
 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

@@ -117,6 +117,20 @@ TYPED_TEST(PrimeFieldTest, CompileTimeEquality)
     static_assert(!(a == f));
 }
 
+TYPED_TEST(PrimeFieldTest, IsZeroOnModulusForm)
+{
+    using F = TypeParam;
+
+    F modulus_form{ F::modulus.data[0], F::modulus.data[1], F::modulus.data[2], F::modulus.data[3] };
+    EXPECT_TRUE(modulus_form.is_zero());
+
+    F prefix_match{ F::modulus.data[0], F::modulus.data[1], F::modulus.data[2], F::modulus.data[3] - 1 };
+    EXPECT_FALSE(prefix_match.is_zero());
+
+    F first_limb_only{ F::modulus.data[0], 0, 0, 0 };
+    EXPECT_FALSE(first_limb_only.is_zero());
+}
+
 TYPED_TEST(PrimeFieldTest, CompileTimeSmallAddSubMul)
 {
     using F = TypeParam;

barretenberg/cpp/src/barretenberg/ecc/groups/affine_element.test.cpp

@@ -190,6 +190,45 @@ template <typename G1> class TestAffineElement : public testing::Test {
         EXPECT_EQ(element(result) == expected, true);
     }
 
+    // Regression test for the large-modulus mixed-addition path: element +/- affine_element must
+    // detect when the affine operand is the infinity sentinel (x = modulus, y = 0). Previously the
+    // operator only checked whether `*this` was infinity, so adding the infinity sentinel to a
+    // normal point fell through to the arithmetic and produced an off-curve garbage result.
+    // operator-=(affine) inherits the bug via its `to_add{other.x, -other.y}` delegation.
+    static void test_mixed_add_infinity_regression()
+    {
+        const element P = element::random_element();
+        const affine_element Q_inf = affine_element::infinity();
+
+        // P (+/-) infinity == P, both as out-of-place and compound-assignment.
+        EXPECT_EQ(P + Q_inf, P);
+        EXPECT_EQ(P - Q_inf, P);
+        {
+            element acc = P;
+            acc += Q_inf;
+            EXPECT_EQ(acc, P);
+        }
+        {
+            element acc = P;
+            acc -= Q_inf;
+            EXPECT_EQ(acc, P);
+        }
+
+        // infinity (+/-) P == +/-P
+        EXPECT_EQ(Q_inf + P, P);
+        EXPECT_EQ(Q_inf - P, -P);
+
+        // *this = infinity, other = infinity must remain infinity (not become {modulus, 0, 1}).
+        element inf_elem = element::zero();
+        ASSERT_TRUE(inf_elem.is_point_at_infinity());
+        EXPECT_TRUE((inf_elem + Q_inf).is_point_at_infinity());
+        EXPECT_TRUE((inf_elem - Q_inf).is_point_at_infinity());
+
+        // The result of mixing a normal point with the infinity sentinel must remain on-curve.
+        EXPECT_TRUE((P + Q_inf).on_curve());
+        EXPECT_TRUE((P - Q_inf).on_curve());
+    }
+
     // Regression test to ensure that the point at infinity is not equal to its coordinate-wise reduction, which may lie
     // on the curve, depending on the y-coordinate.
     static void test_infinity_regression()
@@ -336,6 +375,13 @@ TYPED_TEST(TestAffineElement, AddAffine)
     TestFixture::test_add_affine();
 }
 
+// Regression test for `element +/- affine_element` when the affine operand is the infinity sentinel.
+// Exercises both the large-modulus and small-modulus branches of `element::operator+=(affine)`.
+TYPED_TEST(TestAffineElement, MixedAddInfinityRegression)
+{
+    TestFixture::test_mixed_add_infinity_regression();
+}
+
 TYPED_TEST(TestAffineElement, ReadWrite)
 {
     TestFixture::test_read_and_write();
@@ -417,6 +463,25 @@ TYPED_TEST(TestAffineElement, MulWithEndomorphismMatchesMulWithoutEndomorphism)
     }
 }
 
+// mul_const_time must agree with operator* on every input, including edge cases (0, 1, n-1, low and
+// high Hamming weight).
+TYPED_TEST(TestAffineElement, MulConstTimeMatchesOperatorMul)
+{
+    using element_t = typename TypeParam::element;
+    using Fr = typename TypeParam::Fr;
+    element_t G(element_t::random_element());
+
+    // Edge-case scalars
+    for (Fr s : { Fr::zero(), Fr::one(), -Fr::one(), Fr(2), Fr(uint256_t(1) << 128) }) {
+        EXPECT_EQ(G.mul_const_time(s), G * s);
+    }
+    // Random scalars
+    for (int i = 0; i < 50; ++i) {
+        Fr s = Fr::random_element();
+        EXPECT_EQ(G.mul_const_time(s), G * s);
+    }
+}
+
 // FrCodec is defined only for BN254 and Grumpkin (the two curves whose points appear in transcripts).
 TYPED_TEST(TestAffineElement, FrCodecRoundTrip)
 {

barretenberg/cpp/src/barretenberg/ecc/groups/affine_element_impl.hpp

@@ -100,6 +100,9 @@ template <class Fq, class Fr, class T> constexpr void affine_element<Fq, Fr, T>:
         x.data[1] = Fq::modulus.data[1];
         x.data[2] = Fq::modulus.data[2];
         x.data[3] = Fq::modulus.data[3];
+
+        // Clear y for memory hygiene
+        y = Fq::zero();
     } else {
         (*this).x = Fq::zero();
         (*this).y = Fq::zero();

barretenberg/cpp/src/barretenberg/ecc/groups/element.hpp

@@ -85,6 +85,29 @@ template <class Fq, class Fr, class Params> class alignas(32) element {
     element operator*(const Fr& exponent) const noexcept;
     element operator*=(const Fr& exponent) noexcept;
 
+    /**
+     * @brief Constant-time scalar multiplication intended for secret scalars (e.g. ECDSA / Schnorr nonces).
+     *
+     * Implementation: Montgomery ladder (Montgomery 1987 [1]; SCA-regular form: Joye & Yen,
+     * CHES 2002 [2]) over a fixed iteration count, with Coron's first DPA countermeasure
+     * (CHES 1999 [3]) applied to the scalar: k' = k + r * n for a fresh random 64-bit r sampled
+     * per call. Since n * P = O in the prime-order subgroup, k' * P = k * P; the randomization
+     * decorrelates the per-bit timing trace across signings with the same k.
+     *
+     * [1] P. L. Montgomery, "Speeding the Pollard and Elliptic Curve Methods of Factorization",
+     *     Mathematics of Computation 48 (1987), pp. 243-264.
+     * [2] M. Joye and S.-M. Yen, "The Montgomery Powering Ladder", CHES 2002, LNCS 2523,
+     *     pp. 291-302.
+     * [3] J.-S. Coron, "Resistance against Differential Power Analysis for Elliptic Curve
+     *     Cryptosystems", CHES 1999, LNCS 1717, pp. 292-302.
+     *
+     * @param engine Optional RNG for the blinding factor. If nullptr, uses the global RNG.
+     *
+     * @warning Slower than operator*. Use only when the scalar is secret. For public scalars (MSM,
+     *          public arithmetic), prefer operator*.
+     */
+    element mul_const_time(const Fr& scalar, numeric::RNG* engine = nullptr) const noexcept;
+
     // If you end up implementing this, congrats, you've solved the DL problem!
     // P.S. This is a joke, don't even attempt! 😂
     // constexpr Fr operator/(const element& other) noexcept {}

barretenberg/cpp/src/barretenberg/ecc/groups/element.test.cpp

@@ -296,6 +296,38 @@ template <typename G_> class TestElement : public testing::Test {
         EXPECT_EQ(inf_element.is_point_at_infinity(), true);
     }
 
+    static void test_infinity_canonical_form()
+    {
+        // affine_element: infinity() (value-init then self_set_infinity) and set_infinity() applied
+        // to a random point (copy of a non-trivial state then self_set_infinity) must match.
+        const affine_element inf_from_factory = affine_element::infinity();
+        const affine_element inf_from_random = affine_element(element::random_element()).set_infinity();
+
+        EXPECT_TRUE(inf_from_factory.is_point_at_infinity());
+        EXPECT_TRUE(inf_from_random.is_point_at_infinity());
+
+        EXPECT_EQ(inf_from_factory.y, Fq::zero());
+        EXPECT_EQ(inf_from_random.y, Fq::zero());
+        EXPECT_EQ(inf_from_factory.x, inf_from_random.x);
+        EXPECT_EQ(inf_from_factory.y, inf_from_random.y);
+
+        // element: infinity() (value-init), zero() (default-init, indeterminate y/z storage), and
+        // set_infinity() applied to a random point must all match.
+        const element inf_elem_factory = element::infinity();
+        const element inf_elem_zero = element::zero();
+        const element inf_elem_set = element::random_element().set_infinity();
+
+        EXPECT_TRUE(inf_elem_factory.is_point_at_infinity());
+        EXPECT_TRUE(inf_elem_zero.is_point_at_infinity());
+        EXPECT_TRUE(inf_elem_set.is_point_at_infinity());
+
+        for (const element& e : { inf_elem_factory, inf_elem_zero, inf_elem_set }) {
+            EXPECT_EQ(e.y, Fq::zero());
+            EXPECT_EQ(e.z, Fq::zero());
+            EXPECT_EQ(e.x, inf_elem_factory.x);
+        }
+    }
+
     static void test_derive_generators()
     {
         constexpr size_t num_generators = 128;
@@ -407,6 +439,11 @@ TYPED_TEST(TestElement, Infinity)
     TestFixture::test_infinity();
 }
 
+TYPED_TEST(TestElement, InfinityCanonicalForm)
+{
+    TestFixture::test_infinity_canonical_form();
+}
+
 TYPED_TEST(TestElement, DeriveGenerators)
 {
     if constexpr (!std::is_same_v<typename TestFixture::G, bb::g2>) {