fix: Resolution of bugs from bigfield audits (#9547)

This PR resolves Critical, High, Medium, Low and some informational issues from the bigfield test audits by ZKSecurity, Zellic and Spearbit --------- Co-authored-by: Sarkoxed <sarkoxed2013@yandex.ru>
AztecProtocol · Oct 31, 2024 · feace70 · feace70
1 parent 392114a
commit feace70
Show file tree

Hide file tree

Showing 24 changed files with 991 additions and 456 deletions.
diff --git a/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp b/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp
@@ -611,22 +611,20 @@ TEST(UltraCircuitConstructor, NonNativeFieldMultiplication)
 
     const auto split_into_limbs = [&](const uint512_t& input) {
         constexpr size_t NUM_BITS = 68;
-        std::array<fr, 5> limbs;
+        std::array<fr, 4> limbs;
         limbs[0] = input.slice(0, NUM_BITS).lo;
         limbs[1] = input.slice(NUM_BITS * 1, NUM_BITS * 2).lo;
         limbs[2] = input.slice(NUM_BITS * 2, NUM_BITS * 3).lo;
         limbs[3] = input.slice(NUM_BITS * 3, NUM_BITS * 4).lo;
-        limbs[4] = fr(input.lo);
         return limbs;
     };
 
-    const auto get_limb_witness_indices = [&](const std::array<fr, 5>& limbs) {
-        std::array<uint32_t, 5> limb_indices;
+    const auto get_limb_witness_indices = [&](const std::array<fr, 4>& limbs) {
+        std::array<uint32_t, 4> limb_indices;
         limb_indices[0] = circuit_constructor.add_variable(limbs[0]);
         limb_indices[1] = circuit_constructor.add_variable(limbs[1]);
         limb_indices[2] = circuit_constructor.add_variable(limbs[2]);
         limb_indices[3] = circuit_constructor.add_variable(limbs[3]);
-        limb_indices[4] = circuit_constructor.add_variable(limbs[4]);
         return limb_indices;
     };
     const uint512_t BINARY_BASIS_MODULUS = uint512_t(1) << (68 * 4);
@@ -671,22 +669,20 @@ TEST(UltraCircuitConstructor, NonNativeFieldMultiplicationSortCheck)
 
     const auto split_into_limbs = [&](const uint512_t& input) {
         constexpr size_t NUM_BITS = 68;
-        std::array<fr, 5> limbs;
+        std::array<fr, 4> limbs;
         limbs[0] = input.slice(0, NUM_BITS).lo;
         limbs[1] = input.slice(NUM_BITS * 1, NUM_BITS * 2).lo;
         limbs[2] = input.slice(NUM_BITS * 2, NUM_BITS * 3).lo;
         limbs[3] = input.slice(NUM_BITS * 3, NUM_BITS * 4).lo;
-        limbs[4] = fr(input.lo);
         return limbs;
     };
 
-    const auto get_limb_witness_indices = [&](const std::array<fr, 5>& limbs) {
-        std::array<uint32_t, 5> limb_indices;
+    const auto get_limb_witness_indices = [&](const std::array<fr, 4>& limbs) {
+        std::array<uint32_t, 4> limb_indices;
         limb_indices[0] = circuit_constructor.add_variable(limbs[0]);
         limb_indices[1] = circuit_constructor.add_variable(limbs[1]);
         limb_indices[2] = circuit_constructor.add_variable(limbs[2]);
         limb_indices[3] = circuit_constructor.add_variable(limbs[3]);
-        limb_indices[4] = circuit_constructor.add_variable(limbs[4]);
         return limb_indices;
     };
     const uint512_t BINARY_BASIS_MODULUS = uint512_t(1) << (68 * 4);

diff --git a/barretenberg/cpp/src/barretenberg/dsl/CMakeLists.txt b/barretenberg/cpp/src/barretenberg/dsl/CMakeLists.txt
@@ -13,7 +13,7 @@ set(DSL_DEPENDENCIES
     stdlib_schnorr
     stdlib_honk_verifier)
 
-if (NOT WASM)
+if (NOT WASM AND NOT DISABLE_AZTEC_VM)
     list(APPEND DSL_DEPENDENCIES libdeflate::libdeflate_static vm)
 endif()
 

diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/recursion_constraint.cpp
@@ -114,11 +114,12 @@ AggregationObjectIndices create_recursion_constraints(Builder& builder,
     if (!inner_aggregation_indices_all_zero) {
         std::array<bn254::BaseField, 4> aggregation_elements;
         for (size_t i = 0; i < 4; ++i) {
-            aggregation_elements[i] =
-                bn254::BaseField(field_ct::from_witness_index(&builder, aggregation_input[4 * i]),
-                                 field_ct::from_witness_index(&builder, aggregation_input[4 * i + 1]),
-                                 field_ct::from_witness_index(&builder, aggregation_input[4 * i + 2]),
-                                 field_ct::from_witness_index(&builder, aggregation_input[4 * i + 3]));
+            aggregation_elements[i] = bn254::BaseField::construct_from_limbs(
+                field_ct::from_witness_index(&builder, aggregation_input[4 * i]),
+                field_ct::from_witness_index(&builder, aggregation_input[4 * i + 1]),
+                field_ct::from_witness_index(&builder, aggregation_input[4 * i + 2]),
+                field_ct::from_witness_index(&builder, aggregation_input[4 * i + 3]));
+
             aggregation_elements[i].assert_is_in_field();
         }
         // If we have a previous aggregation object, assign it to `previous_aggregation` so that it is included

diff --git a/barretenberg/cpp/src/barretenberg/plonk/composer/ultra_composer.test.cpp b/barretenberg/cpp/src/barretenberg/plonk/composer/ultra_composer.test.cpp
@@ -593,22 +593,20 @@ TYPED_TEST(ultra_plonk_composer, non_native_field_multiplication)
 
     const auto split_into_limbs = [&](const uint512_t& input) {
         constexpr size_t NUM_BITS = 68;
-        std::array<fr, 5> limbs;
+        std::array<fr, 4> limbs;
         limbs[0] = input.slice(0, NUM_BITS).lo;
         limbs[1] = input.slice(NUM_BITS * 1, NUM_BITS * 2).lo;
         limbs[2] = input.slice(NUM_BITS * 2, NUM_BITS * 3).lo;
         limbs[3] = input.slice(NUM_BITS * 3, NUM_BITS * 4).lo;
-        limbs[4] = fr(input.lo);
         return limbs;
     };
 
-    const auto get_limb_witness_indices = [&](const std::array<fr, 5>& limbs) {
-        std::array<uint32_t, 5> limb_indices;
+    const auto get_limb_witness_indices = [&](const std::array<fr, 4>& limbs) {
+        std::array<uint32_t, 4> limb_indices;
         limb_indices[0] = builder.add_variable(limbs[0]);
         limb_indices[1] = builder.add_variable(limbs[1]);
         limb_indices[2] = builder.add_variable(limbs[2]);
         limb_indices[3] = builder.add_variable(limbs[3]);
-        limb_indices[4] = builder.add_variable(limbs[4]);
         return limb_indices;
     };
     const uint512_t BINARY_BASIS_MODULUS = uint512_t(1) << (68 * 4);

diff --git a/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa_impl.hpp
@@ -82,8 +82,9 @@ bool_t<Builder> ecdsa_verify_signature(const stdlib::byte_array<Builder>& messag
     // Read more about this at: https://www.derpturkey.com/inherent-malleability-of-ecdsa-signatures/amp/
     s.assert_less_than((Fr::modulus + 1) / 2);
 
-    Fr u1 = z / s;
-    Fr u2 = r / s;
+    // We already checked that s is nonzero
+    Fr u1 = z.div_without_denominator_check(s);
+    Fr u2 = r.div_without_denominator_check(s);
 
     public_key.validate_on_curve();
 

diff --git a/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.cpp b/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.cpp
@@ -70,8 +70,8 @@ UltraRecursiveVerifier_<Flavor>::AggregationObject UltraRecursiveVerifier_<Flavo
                 bigfield_limbs[k] = verification_key->public_inputs[key->recursive_proof_public_input_indices[idx]];
                 idx++;
             }
-            base_field_vals[j] =
-                typename Curve::BaseField(bigfield_limbs[0], bigfield_limbs[1], bigfield_limbs[2], bigfield_limbs[3]);
+            base_field_vals[j] = Curve::BaseField::construct_from_limbs(
+                bigfield_limbs[0], bigfield_limbs[1], bigfield_limbs[2], bigfield_limbs[3]);
         }
         nested_pairing_points[i] = typename Curve::Group(base_field_vals[0], base_field_vals[1]);
     }

diff --git a/...nberg/cpp/src/barretenberg/stdlib/plonk_recursion/aggregation_state/aggregation_state.hpp b/...nberg/cpp/src/barretenberg/stdlib/plonk_recursion/aggregation_state/aggregation_state.hpp
@@ -109,11 +109,11 @@ aggregation_state<Curve> convert_witness_indices_to_agg_obj(Builder& builder,
 {
     std::array<typename Curve::BaseField, 4> aggregation_elements;
     for (size_t i = 0; i < 4; ++i) {
-        aggregation_elements[i] =
-            typename Curve::BaseField(Curve::ScalarField::from_witness_index(&builder, witness_indices[4 * i]),
-                                      Curve::ScalarField::from_witness_index(&builder, witness_indices[4 * i + 1]),
-                                      Curve::ScalarField::from_witness_index(&builder, witness_indices[4 * i + 2]),
-                                      Curve::ScalarField::from_witness_index(&builder, witness_indices[4 * i + 3]));
+        aggregation_elements[i] = Curve::BaseField::construct_from_limbs(
+            Curve::ScalarField::from_witness_index(&builder, witness_indices[4 * i]),
+            Curve::ScalarField::from_witness_index(&builder, witness_indices[4 * i + 1]),
+            Curve::ScalarField::from_witness_index(&builder, witness_indices[4 * i + 2]),
+            Curve::ScalarField::from_witness_index(&builder, witness_indices[4 * i + 3]));
         aggregation_elements[i].assert_is_in_field();
     }
 

diff --git a/barretenberg/cpp/src/barretenberg/stdlib/plonk_recursion/verifier/verifier.hpp b/barretenberg/cpp/src/barretenberg/stdlib/plonk_recursion/verifier/verifier.hpp
@@ -345,7 +345,7 @@ aggregation_state<Curve> verify_proof_(typename Curve::Builder* context,
                 l1.create_range_constraint(fq_ct::NUM_LIMB_BITS, "l1");
                 l2.create_range_constraint(fq_ct::NUM_LIMB_BITS, "l2");
                 l3.create_range_constraint(fq_ct::NUM_LAST_LIMB_BITS, "l3");
-                return fq_ct(l0, l1, l2, l3, false);
+                return fq_ct::unsafe_construct_from_limbs(l0, l1, l2, l3, false);
             };
 
         fr_ct recursion_separator_challenge = transcript.get_challenge_field_element("separator", 2);