commit 61a21e7ec5ab9cd81bdc7616d8a110d7ccf58ea3
author David Benjamin <davidben@google.com> Thu Jul 08 16:07:07 2021 -0400
committer Adam Langley <agl@google.com> Fri Jul 09 16:58:00 2021 +0000
tree 1bd3ff8c5242e01347d69676e51bc62c10f27138
parent ad5db965845a12fbd69bd23eab00dd6b9e5998f6

Fix sign bit in BN_div if numerator and quotient alias.

See also f8fc0e35e0b1813af15887d42e17b7d5537bb86c from upstream, though
our BN_divs have diverged slightly.

Change-Id: I49fa4f0a5c730d34e6f41f724f1afe3685470712
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48426
Reviewed-by: Adam Langley <agl@google.com>

crypto/fipsmodule/bn/bn_test.cc

diff --git a/crypto/fipsmodule/bn/bn_test.cc b/crypto/fipsmodule/bn/bn_test.cc
index 9791437..1b42d9c 100644
--- a/crypto/fipsmodule/bn/bn_test.cc
+++ b/crypto/fipsmodule/bn/bn_test.cc
@@ -556,6 +556,19 @@
   EXPECT_BIGNUMS_EQUAL("A / B", quotient.get(), ret.get());
   EXPECT_BIGNUMS_EQUAL("A % B", remainder.get(), ret2.get());
 
+  ASSERT_TRUE(BN_copy(ret.get(), a.get()));
+  ASSERT_TRUE(BN_copy(ret2.get(), b.get()));
+  ASSERT_TRUE(BN_div(ret.get(), ret2.get(), ret.get(), ret2.get(), ctx));
+  EXPECT_BIGNUMS_EQUAL("A / B (in-place)", quotient.get(), ret.get());
+  EXPECT_BIGNUMS_EQUAL("A % B (in-place)", remainder.get(), ret2.get());
+
+  ASSERT_TRUE(BN_copy(ret2.get(), a.get()));
+  ASSERT_TRUE(BN_copy(ret.get(), b.get()));
+  ASSERT_TRUE(BN_div(ret.get(), ret2.get(), ret2.get(), ret.get(), ctx));
+  EXPECT_BIGNUMS_EQUAL("A / B (in-place, swapped)", quotient.get(), ret.get());
+  EXPECT_BIGNUMS_EQUAL("A % B (in-place, swapped)", remainder.get(),
+                       ret2.get());
+
   ASSERT_TRUE(BN_mul(ret.get(), quotient.get(), b.get(), ctx));
   ASSERT_TRUE(BN_add(ret.get(), ret.get(), remainder.get()));
   EXPECT_BIGNUMS_EQUAL("Quotient * B + Remainder", a.get(), ret.get());

crypto/fipsmodule/bn/div.c

diff --git a/crypto/fipsmodule/bn/div.c b/crypto/fipsmodule/bn/div.c
index 333c770..6ee6dbd 100644
--- a/crypto/fipsmodule/bn/div.c
+++ b/crypto/fipsmodule/bn/div.c
@@ -285,8 +285,10 @@
   // pointer to the 'top' of snum
   wnump = &(snum->d[num_n - 1]);
 
-  // Setup to 'res'
-  res->neg = (numerator->neg ^ divisor->neg);
+  // Setup |res|. |numerator| and |res| may alias, so we save |numerator->neg|
+  // for later.
+  const int numerator_neg = numerator->neg;
+  res->neg = (numerator_neg ^ divisor->neg);
   if (!bn_wexpand(res, loop + 1)) {
     goto err;
   }
@@ -379,14 +381,11 @@
   bn_set_minimal_width(snum);
 
   if (rem != NULL) {
-    // Keep a copy of the neg flag in numerator because if |rem| == |numerator|
-    // |BN_rshift| will overwrite it.
-    int neg = numerator->neg;
     if (!BN_rshift(rem, snum, norm_shift)) {
       goto err;
     }
     if (!BN_is_zero(rem)) {
-      rem->neg = neg;
+      rem->neg = numerator_neg;
     }
   }