From b0d6399f60760e25a55ec9e8e95a1ad322b74b22 Mon Sep 17 00:00:00 2001
From: Timothy Flynn <trflynn89@pm.me>
Date: Sun, 6 Feb 2022 13:17:34 +0000
Subject: [PATCH] LibCrypto: Do not allow signed big integers to be negative
 zero

If a big integer were to become negative zero, set the sign to instead
be positive. This prevents odd scenarios where users of signed big ints
would falsely think the result of some big int arithmetic is negative.
---
 Tests/LibCrypto/TestBigInteger.cpp            | 16 +++++++++++++++
 .../LibCrypto/BigInt/SignedBigInteger.h       | 20 +++++++++++++++++--
 .../LibCrypto/BigInt/UnsignedBigInteger.cpp   | 10 ++++++++++
 .../LibCrypto/BigInt/UnsignedBigInteger.h     |  1 +
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/Tests/LibCrypto/TestBigInteger.cpp b/Tests/LibCrypto/TestBigInteger.cpp
index 7af6eb4a1d4..94beca95539 100644
--- a/Tests/LibCrypto/TestBigInteger.cpp
+++ b/Tests/LibCrypto/TestBigInteger.cpp
@@ -642,3 +642,19 @@ TEST_CASE(test_signed_multiplication_with_two_big_numbers)
     EXPECT_EQ(result.unsigned_value().words(), expected_results);
     EXPECT(result.is_negative());
 }
+
+TEST_CASE(test_negative_zero_is_not_allowed)
+{
+    Crypto::SignedBigInteger zero(Crypto::UnsignedBigInteger(0), true);
+    EXPECT(!zero.is_negative());
+
+    zero.negate();
+    EXPECT(!zero.is_negative());
+
+    Crypto::SignedBigInteger positive_five(Crypto::UnsignedBigInteger(5), false);
+    Crypto::SignedBigInteger negative_five(Crypto::UnsignedBigInteger(5), true);
+    zero = positive_five.plus(negative_five);
+
+    EXPECT(zero.unsigned_value().is_zero());
+    EXPECT(!zero.is_negative());
+}
diff --git a/Userland/Libraries/LibCrypto/BigInt/SignedBigInteger.h b/Userland/Libraries/LibCrypto/BigInt/SignedBigInteger.h
index c8850fd5abf..5b5468a728a 100644
--- a/Userland/Libraries/LibCrypto/BigInt/SignedBigInteger.h
+++ b/Userland/Libraries/LibCrypto/BigInt/SignedBigInteger.h
@@ -25,6 +25,7 @@ public:
         : m_sign(sign)
         , m_unsigned_data(move(unsigned_data))
     {
+        ensure_sign_is_valid();
     }
 
     explicit SignedBigInteger(UnsignedBigInteger unsigned_data)
@@ -72,9 +73,18 @@ public:
     const Vector<u32, STARTING_WORD_SIZE> words() const { return m_unsigned_data.words(); }
     bool is_negative() const { return m_sign; }
 
-    void negate() { m_sign = !m_sign; }
+    void negate()
+    {
+        if (!m_unsigned_data.is_zero())
+            m_sign = !m_sign;
+    }
+
+    void set_to_0()
+    {
+        m_unsigned_data.set_to_0();
+        m_sign = false;
+    }
 
-    void set_to_0() { m_unsigned_data.set_to_0(); }
     void set_to(i32 other)
     {
         m_unsigned_data.set_to((u32)other);
@@ -129,6 +139,12 @@ public:
     bool operator>(const UnsignedBigInteger& other) const;
 
 private:
+    void ensure_sign_is_valid()
+    {
+        if (m_sign && m_unsigned_data.is_zero())
+            m_sign = false;
+    }
+
     bool m_sign { false };
     UnsignedBigInteger m_unsigned_data;
 };
diff --git a/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp b/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp
index ef560153957..a8082721d1d 100644
--- a/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp
+++ b/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.cpp
@@ -145,6 +145,16 @@ void UnsignedBigInteger::set_to(const UnsignedBigInteger& other)
     m_cached_hash = 0;
 }
 
+bool UnsignedBigInteger::is_zero() const
+{
+    for (size_t i = 0; i < length(); ++i) {
+        if (m_words[i] != 0)
+            return false;
+    }
+
+    return true;
+}
+
 size_t UnsignedBigInteger::trimmed_length() const
 {
     if (!m_cached_trimmed_length.has_value()) {
diff --git a/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.h b/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.h
index 14c9a90b4c6..3f91aa8d514 100644
--- a/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.h
+++ b/Userland/Libraries/LibCrypto/BigInt/UnsignedBigInteger.h
@@ -72,6 +72,7 @@ public:
         m_cached_hash = 0;
     }
 
+    bool is_zero() const;
     bool is_odd() const { return m_words.size() && (m_words[0] & 1); }
     bool is_invalid() const { return m_is_invalid; }