bn: fix occurances of negative zero
authorGeoff Thorpe <geoff@openssl.org>
Thu, 6 Oct 2016 15:04:56 +0000 (10:04 -0500)
committerRichard Levitte <levitte@openssl.org>
Wed, 1 Feb 2017 01:06:39 +0000 (02:06 +0100)
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)

crypto/bn/bn_div.c
crypto/bn/bn_mul.c
crypto/bn/bn_shift.c
crypto/bn/bn_word.c

index 99abf35..5e620b2 100644 (file)
@@ -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' */
     wnump = &(snum->d[num_n - 1]);
 
     /* Setup to 'res' */
-    res->neg = (num->neg ^ divisor->neg);
     if (!bn_wexpand(res, (loop + 1)))
         goto err;
     if (!bn_wexpand(res, (loop + 1)))
         goto err;
+    res->neg = (num->neg ^ divisor->neg);
     res->top = loop - no_branch;
     resp = &(res->d[loop - 1]);
 
     res->top = loop - no_branch;
     resp = &(res->d[loop - 1]);
 
index 4c39d40..4a0a950 100644 (file)
@@ -857,7 +857,6 @@ int BN_mul(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx)
             goto err;
     } else
         rr = r;
             goto err;
     } else
         rr = r;
-    rr->neg = a->neg ^ b->neg;
 
 #if defined(BN_MUL_COMBA) || defined(BN_RECURSION)
     i = al - bl;
 
 #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
 #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;
     bn_correct_top(rr);
     if (r != rr && BN_copy(r, rr) == NULL)
         goto err;
index 9907b82..b320602 100644 (file)
@@ -92,10 +92,10 @@ int BN_lshift(BIGNUM *r, const BIGNUM *a, int n)
         return 0;
     }
 
         return 0;
     }
 
-    r->neg = a->neg;
     nw = n / BN_BITS2;
     if (bn_wexpand(r, a->top + nw + 1) == NULL)
         return (0);
     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;
     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) {
     }
     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);
         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 */
     } 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 ((l = (l >> rb) & BN_MASK2))
             *(t) = l;
     }
+    if (!r->top)
+        r->neg = 0; /* don't allow negative zero */
     bn_check_top(r);
     return (1);
 }
     bn_check_top(r);
     return (1);
 }
index a34244c..1af13a5 100644 (file)
@@ -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 > 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);
 }
     bn_check_top(a);
     return (ret);
 }