bn: fix occurances of negative zero

The BIGNUM behaviour is supposed to be "consistent" when going into and
out of APIs, where "consistent" means 'top' is set minimally and that
'neg' (negative) is not set if the BIGNUM is zero (which is iff 'top' is
zero, due to the previous point).

The BN_DEBUG testing (make test) caught the cases that this patch
corrects.

Note, bn_correct_top() could have been used instead, but that is intended
for where 'top' is expected to (sometimes) require adjustment after direct
word-array manipulation, and so is heavier-weight. Here, we are just
catching the negative-zero case, so we test and correct for that
explicitly, in-place.

Change-Id: Iddefbd3c28a13d935648932beebcc765d5b85ae7
Signed-off-by: Geoff Thorpe <geoff@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1672)
(cherry picked from commit 38d1b3cc0271008b8bd130a2c4b442775b028a08)

diff --git a/crypto/bn/bn_div.c b/crypto/bn/bn_div.c
index 99abf35c418f48164884770f5e33ac9bb4c9884f..5e620b2096ca0366fa0e2e4b9d4ca9964a05a3b5 100644 (file)
--- a/crypto/bn/bn_div.c
+++ b/crypto/bn/bn_div.c
@@ -254,9 +254,9 @@ int BN_div(BIGNUM *dv, BIGNUM *rm, const BIGNUM *num, const BIGNUM *divisor,
     wnump = &(snum->d[num_n - 1]);
 
     /* Setup to 'res' */
-    res->neg = (num->neg ^ divisor->neg);
     if (!bn_wexpand(res, (loop + 1)))
         goto err;
+    res->neg = (num->neg ^ divisor->neg);
     res->top = loop - no_branch;
     resp = &(res->d[loop - 1]);
 

diff --git a/crypto/bn/bn_mul.c b/crypto/bn/bn_mul.c
index 4c39d404b5b03d0b22dfd2015a9cfb07b5166399..4a0a9505b70e6bb694824d441ad5c0d250efefbf 100644 (file)
--- a/crypto/bn/bn_mul.c
+++ b/crypto/bn/bn_mul.c
@@ -857,7 +857,6 @@ int BN_mul(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx)
             goto err;
     } else
         rr = r;
-    rr->neg = a->neg ^ b->neg;
 
 #if defined(BN_MUL_COMBA) || defined(BN_RECURSION)
     i = al - bl;
@@ -969,6 +968,7 @@ int BN_mul(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx)
 #if defined(BN_MUL_COMBA) || defined(BN_RECURSION)
  end:
 #endif
+    rr->neg = a->neg ^ b->neg;
     bn_correct_top(rr);
     if (r != rr && BN_copy(r, rr) == NULL)
         goto err;

diff --git a/crypto/bn/bn_shift.c b/crypto/bn/bn_shift.c
index 9907b82fa2eb994b10b611358de0d69a07f836c4..b3206028713c44d6faa31cc3460f2f308a552a82 100644 (file)
--- a/crypto/bn/bn_shift.c
+++ b/crypto/bn/bn_shift.c
@@ -92,10 +92,10 @@ int BN_lshift(BIGNUM *r, const BIGNUM *a, int n)
         return 0;
     }
 
-    r->neg = a->neg;
     nw = n / BN_BITS2;
     if (bn_wexpand(r, a->top + nw + 1) == NULL)
         return (0);
+    r->neg = a->neg;
     lb = n % BN_BITS2;
     rb = BN_BITS2 - lb;
     f = a->d;
@@ -140,9 +140,9 @@ int BN_rshift(BIGNUM *r, const BIGNUM *a, int n)
     }
     i = (BN_num_bits(a) - n + (BN_BITS2 - 1)) / BN_BITS2;
     if (r != a) {
-        r->neg = a->neg;
         if (bn_wexpand(r, i) == NULL)
             return (0);
+        r->neg = a->neg;
     } else {
         if (n == 0)
             return 1;           /* or the copying loop will go berserk */
@@ -166,6 +166,8 @@ int BN_rshift(BIGNUM *r, const BIGNUM *a, int n)
         if ((l = (l >> rb) & BN_MASK2))
             *(t) = l;
     }
+    if (!r->top)
+        r->neg = 0; /* don't allow negative zero */
     bn_check_top(r);
     return (1);
 }

diff --git a/crypto/bn/bn_word.c b/crypto/bn/bn_word.c
index a34244c4aded58d3012ee7976420886326e86a13..1af13a53fb1c81e93328da7eb0b828759a5efbad 100644 (file)
--- a/crypto/bn/bn_word.c
+++ b/crypto/bn/bn_word.c
@@ -89,6 +89,8 @@ BN_ULONG BN_div_word(BIGNUM *a, BN_ULONG w)
     if ((a->top > 0) && (a->d[a->top - 1] == 0))
         a->top--;
     ret >>= j;
+    if (!a->top)
+        a->neg = 0; /* don't allow negative zero */
     bn_check_top(a);
     return (ret);
 }