Fix for informational issue Operator overloading for division is dang…

…erous
AztecProtocol · Oct 30, 2024 · 8d8c462 · 8d8c462
1 parent 17fe293
commit 8d8c462
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 5 deletions.
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/primitives/bigfield/bigfield.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp
@@ -357,6 +357,7 @@ template <typename Builder, typename T> class bigfield {
                                  bool check_for_zero);
 
     static bigfield div_without_denominator_check(const std::vector<bigfield>& numerators, const bigfield& denominator);
+    bigfield div_without_denominator_check(const bigfield& denominator);
     static bigfield div_check_denominator_nonzero(const std::vector<bigfield>& numerators, const bigfield& denominator);
 
     bigfield conditional_negate(const bool_t<Builder>& predicate) const;

diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp
@@ -776,16 +776,16 @@ bigfield<Builder, T> bigfield<Builder, T>::operator*(const bigfield& other) cons
 }
 
 /**
- * Division operator. Doesn't create constraints for b!=0, which can lead to vulnerabilities. If you need a safer
- *variant use div_check_denominator_nonzero.
+ * Division operator. Create constraints for b!=0 by default. If you need a variant
+ *without the zero check,  use div_without_denominator_check.
  *
  * To evaluate (a / b = c mod p), we instead evaluate (c * b = a mod p).
  **/
 template <typename Builder, typename T>
 bigfield<Builder, T> bigfield<Builder, T>::operator/(const bigfield& other) const
 {
 
-    return internal_div({ *this }, other, false);
+    return internal_div({ *this }, other, true);
 }
 /**
  * @brief Create constraints for summing these terms
@@ -911,6 +911,12 @@ bigfield<Builder, T> bigfield<Builder, T>::div_without_denominator_check(const s
     return internal_div(numerators, denominator, false);
 }
 
+template <typename Builder, typename T>
+bigfield<Builder, T> bigfield<Builder, T>::div_without_denominator_check(const bigfield& denominator)
+{
+    return internal_div({ *this }, denominator, false);
+}
+
 /**
  * Div method with constraints for denominator!=0.
  *