Fix off-by-one in BN_rand

If BN_rand is called with |bits| set to 1 and |top| set to 1 then a 1 byte
buffer overflow can occur. There are no such instances within the OpenSSL at
the moment.

Thanks to Mateusz Kocielski (LogicalTrust), Marek Kroemeke, Filip Palian for
discovering and reporting this issue.

Reviewed-by: Kurt Roeckx <kurt@openssl.org>

diff --git a/crypto/bn/bn.h b/crypto/bn/bn.h
index 7311c0a8cfb7a9918cf0d02ff395aba83eba9a6c..5696965e9a09d070726ee333cc75f23ea4df7d05 100644 (file)
--- a/crypto/bn/bn.h
+++ b/crypto/bn/bn.h
@@ -916,6 +916,7 @@ void ERR_load_BN_strings(void);
 # define BN_R_ARG2_LT_ARG3                                100
 # define BN_R_BAD_RECIPROCAL                              101
 # define BN_R_BIGNUM_TOO_LONG                             114
+# define BN_R_BITS_TOO_SMALL                              118
 # define BN_R_CALLED_WITH_EVEN_MODULUS                    102
 # define BN_R_DIV_BY_ZERO                                 103
 # define BN_R_ENCODING_ERROR                              104

diff --git a/crypto/bn/bn_err.c b/crypto/bn/bn_err.c
index a9b7f5193b2c5cbd94d4b6268f0841822eb168f3..e7a703826ee57bc1ad3f5a670925382c565674ff 100644 (file)
--- a/crypto/bn/bn_err.c
+++ b/crypto/bn/bn_err.c
@@ -1,6 +1,6 @@
 /* crypto/bn/bn_err.c */
 /* ====================================================================
- * Copyright (c) 1999-2007 The OpenSSL Project.  All rights reserved.
+ * Copyright (c) 1999-2015 The OpenSSL Project.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -119,6 +119,7 @@ static ERR_STRING_DATA BN_str_reasons[] = {
     {ERR_REASON(BN_R_ARG2_LT_ARG3), "arg2 lt arg3"},
     {ERR_REASON(BN_R_BAD_RECIPROCAL), "bad reciprocal"},
     {ERR_REASON(BN_R_BIGNUM_TOO_LONG), "bignum too long"},
+    {ERR_REASON(BN_R_BITS_TOO_SMALL), "bits too small"},
     {ERR_REASON(BN_R_CALLED_WITH_EVEN_MODULUS), "called with even modulus"},
     {ERR_REASON(BN_R_DIV_BY_ZERO), "div by zero"},
     {ERR_REASON(BN_R_ENCODING_ERROR), "encoding error"},

diff --git a/crypto/bn/bn_rand.c b/crypto/bn/bn_rand.c
index 9e78d4d42a14908999dfe5354a71800b7338fdb2..f9fb2e9e45e01b9438a1c0bc5c4fb82311fbe363 100644 (file)
--- a/crypto/bn/bn_rand.c
+++ b/crypto/bn/bn_rand.c
@@ -121,6 +121,11 @@ static int bnrand(int pseudorand, BIGNUM *rnd, int bits, int top, int bottom)
     int ret = 0, bit, bytes, mask;
     time_t tim;
 
+    if (bits < 0 || (bits == 1 && top > 0)) {
+        BNerr(BN_F_BNRAND, BN_R_BITS_TOO_SMALL);
+        return 0;
+    }
+
     if (bits == 0) {
         BN_zero(rnd);
         return 1;
@@ -169,7 +174,7 @@ static int bnrand(int pseudorand, BIGNUM *rnd, int bits, int top, int bottom)
     }
 #endif
 
-    if (top != -1) {
+    if (top >= 0) {
         if (top) {
             if (bit == 0) {
                 buf[0] = 1;

diff --git a/doc/crypto/BN_rand.pod b/doc/crypto/BN_rand.pod
index d6b975ccf64385a96b5c880e4ddab4a5cf725dda..bd6bc8632394edb05908556338aa82dd7b1f977d 100644 (file)
--- a/doc/crypto/BN_rand.pod
+++ b/doc/crypto/BN_rand.pod
@@ -24,7 +24,8 @@ most significant bit of the random number can be zero. If B<top> is 0,
 it is set to 1, and if B<top> is 1, the two most significant bits of
 the number will be set to 1, so that the product of two such random
 numbers will always have 2*B<bits> length.  If B<bottom> is true, the
-number will be odd.
+number will be odd. The value of B<bits> must be zero or greater. If B<bits> is
+1 then B<top> cannot also be 1.
 
 BN_pseudo_rand() does the same, but pseudo-random numbers generated by
 this function are not necessarily unpredictable. They can be used for