From b484b040e39e03efbc889a204f5fd2406d7b04b5 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 19 May 2015 16:03:02 +0100 Subject: [PATCH] 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 --- crypto/bn/bn.h | 1 + crypto/bn/bn_err.c | 3 ++- crypto/bn/bn_rand.c | 7 ++++++- doc/crypto/BN_rand.pod | 3 ++- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/crypto/bn/bn.h b/crypto/bn/bn.h index da2a0e1821..47d8c71d9e 100644 --- a/crypto/bn/bn.h +++ b/crypto/bn/bn.h @@ -934,6 +934,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 a9b7f5193b..e7a703826e 100644 --- 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 9e78d4d42a..f9fb2e9e45 100644 --- 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 81f93c2eb3..3b2796c5df 100644 --- 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 is 0, it is set to 1, and if B 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 length. If B is true, the -number will be odd. +number will be odd. The value of B must be zero or greater. If B is +1 then B 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 -- 2.25.1