From 8d8c4627f80d0a20207525c70417f6bbbfa48db1 Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Thu, 26 Sep 2024 17:03:01 +0000 Subject: [PATCH] Fix for informational issue Operator overloading for division is dangerous --- .../stdlib/encryption/ecdsa/ecdsa_impl.hpp | 5 +++-- .../stdlib/primitives/bigfield/bigfield.hpp | 1 + .../stdlib/primitives/bigfield/bigfield_impl.hpp | 12 +++++++++--- 3 files changed, 13 insertions(+), 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 index 02c69d8f770b..aefc1a2316b9 100644 --- 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 ecdsa_verify_signature(const stdlib::byte_array& 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 index bab6dbcfd7b8..1276f229db7f 100644 --- 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 class bigfield { bool check_for_zero); static bigfield div_without_denominator_check(const std::vector& numerators, const bigfield& denominator); + bigfield div_without_denominator_check(const bigfield& denominator); static bigfield div_check_denominator_nonzero(const std::vector& numerators, const bigfield& denominator); bigfield conditional_negate(const bool_t& 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 index c5d7ff87fec0..a0e9eb058adc 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -776,8 +776,8 @@ bigfield bigfield::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). **/ @@ -785,7 +785,7 @@ template bigfield bigfield::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 bigfield::div_without_denominator_check(const s return internal_div(numerators, denominator, false); } +template +bigfield bigfield::div_without_denominator_check(const bigfield& denominator) +{ + return internal_div({ *this }, denominator, false); +} + /** * Div method with constraints for denominator!=0. *