Fix out-of-bounds read in BN_mod_exp_mont_consttime.

bn_get_bits5 always reads two bytes, even when it doesn't need to. For some
sizes of |p|, this can result in reading just past the edge of the array.
Unroll the first iteration of the loop and avoid reading out of bounds.

Replace bn_get_bits5 altogether in C as it's not doing anything interesting.

Change-Id: Ibcc8cea7d9c644a2639445396455da47fe869a5c
Reviewed-on: https://boringssl-review.googlesource.com/1393
Reviewed-by: Adam Langley <agl@google.com>

crypto/bn/asm/x86_64-mont5.pl

@@ -3221,20 +3221,6 @@ my $STRIDE=2**5*8;
 my $N=$STRIDE/4;
 
 $code.=<<___;
-.globl	bn_get_bits5
-.type	bn_get_bits5,\@abi-omnipotent
-.align	16
-bn_get_bits5:
-	mov	$inp,%r10
-	mov	$num,%ecx
-	shr	\$3,$num
-	movzw	(%r10,$num),%eax
-	and	\$7,%ecx
-	shrl	%cl,%eax
-	and	\$31,%eax
-	ret
-.size	bn_get_bits5,.-bn_get_bits5
-
 .globl	bn_scatter5
 .type	bn_scatter5,\@abi-omnipotent
 .align	16

crypto/bn/exponentiation.c

@@ -108,6 +108,8 @@
 
 #include <openssl/bn.h>
 
+#include <assert.h>
+
 #include <openssl/cpu.h>
 #include <openssl/err.h>
 #include <openssl/mem.h>
@@ -991,7 +993,6 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
     void bn_power5(BN_ULONG * rp, const BN_ULONG * ap, const void * table,
                    const BN_ULONG * np, const BN_ULONG * n0, int num,
                    int power);
-    int bn_get_bits5(const BN_ULONG * ap, int off);
     int bn_from_montgomery(BN_ULONG * rp, const BN_ULONG * ap,
                            const BN_ULONG * not_used, const BN_ULONG * np,
                            const BN_ULONG * n0, int num);
@@ -1046,10 +1047,14 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
       wvalue = (wvalue << 1) + BN_is_bit_set(p, bits);
     bn_gather5(tmp.d, top, powerbuf, wvalue);
 
+    /* At this point |bits| is 4 mod 5 and at least -1. (|bits| is the first bit
+     * that has not been read yet.) */
+    assert(bits >= -1 && (bits == -1 || bits % 5 == 4));
+
     /* Scan the exponent one window at a time starting from the most
      * significant bits.
      */
-    if (top & 7)
+    if (top & 7) {
       while (bits >= 0) {
         for (wvalue = 0, i = 0; i < 5; i++, bits--)
           wvalue = (wvalue << 1) + BN_is_bit_set(p, bits);
@@ -1061,9 +1066,32 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
         bn_mul_mont(tmp.d, tmp.d, tmp.d, np, n0, top);
         bn_mul_mont_gather5(tmp.d, tmp.d, powerbuf, np, n0, top, wvalue);
       }
-    else {
+    } else {
+      const uint8_t *p_bytes = (const uint8_t *)p->d;
+      int max_bits = p->top * BN_BITS2;
+      assert(bits < max_bits);
+      /* |p = 0| has been handled as a special case, so |max_bits| is at least
+       * one word. */
+      assert(max_bits >= 64);
+
+      /* If the first bit to be read lands in the last byte, unroll the first
+       * iteration to avoid reading past the bounds of |p->d|. (After the first
+       * iteration, we are guaranteed to be past the last byte.) Note |bits|
+       * here is the top bit, inclusive. */
+      if (bits - 4 >= max_bits - 8) {
+        /* Read five bits from |bits-4| through |bits|, inclusive. */
+        wvalue = p_bytes[p->top * BN_BYTES - 1];
+        wvalue >>= (bits - 4) & 7;
+        wvalue &= 0x1f;
+        bits -= 5;
+        bn_power5(tmp.d, tmp.d, powerbuf, np2, n0, top, wvalue);
+      }
       while (bits >= 0) {
-        wvalue = bn_get_bits5(p->d, bits - 4);
+        /* Read five bits from |bits-4| through |bits|, inclusive. */
+        int first_bit = bits - 4;
+        wvalue = *(const uint16_t *) (p_bytes + (first_bit >> 3));
+        wvalue >>= first_bit & 7;
+        wvalue &= 0x1f;
         bits -= 5;
         bn_power5(tmp.d, tmp.d, powerbuf, np2, n0, top, wvalue);
       }