From 726b5e71329865d14e46e1eb96c986e3e373bbfd Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 19 May 2015 15:19:30 +0100 Subject: [PATCH] Reject negative shifts for BN_rshift and BN_lshift The functions BN_rshift and BN_lshift shift their arguments to the right or left by a specified number of bits. Unpredicatable results (including crashes) can occur if a negative number is supplied for the shift value. Thanks to Mateusz Kocielski (LogicalTrust), Marek Kroemeke and Filip Palian for discovering and reporting this issue. Reviewed-by: Kurt Roeckx (cherry picked from commit 7cc18d8158b5fc2676393d99b51c30c135502107) Conflicts: crypto/bn/bn.h crypto/bn/bn_err.c --- crypto/bn/bn.h | 3 +++ crypto/bn/bn_err.c | 3 +++ crypto/bn/bn_shift.c | 10 ++++++++++ doc/crypto/BN_set_bit.pod | 8 ++++---- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/crypto/bn/bn.h b/crypto/bn/bn.h index d615be62cd..da2a0e1821 100644 --- a/crypto/bn/bn.h +++ b/crypto/bn/bn.h @@ -911,6 +911,7 @@ void ERR_load_BN_strings(void); # define BN_F_BN_GF2M_MOD_SOLVE_QUAD_ARR 135 # define BN_F_BN_GF2M_MOD_SQR 136 # define BN_F_BN_GF2M_MOD_SQRT 137 +# define BN_F_BN_LSHIFT 145 # define BN_F_BN_MOD_EXP2_MONT 118 # define BN_F_BN_MOD_EXP_MONT 109 # define BN_F_BN_MOD_EXP_MONT_CONSTTIME 124 @@ -926,6 +927,7 @@ void ERR_load_BN_strings(void); # define BN_F_BN_NEW 113 # define BN_F_BN_RAND 114 # define BN_F_BN_RAND_RANGE 122 +# define BN_F_BN_RSHIFT 146 # define BN_F_BN_USUB 115 /* Reason codes. */ @@ -939,6 +941,7 @@ void ERR_load_BN_strings(void); # define BN_R_INPUT_NOT_REDUCED 110 # define BN_R_INVALID_LENGTH 106 # define BN_R_INVALID_RANGE 115 +# define BN_R_INVALID_SHIFT 119 # define BN_R_NOT_A_SQUARE 111 # define BN_R_NOT_INITIALIZED 107 # define BN_R_NO_INVERSE 108 diff --git a/crypto/bn/bn_err.c b/crypto/bn/bn_err.c index faa7e226ba..a9b7f5193b 100644 --- a/crypto/bn/bn_err.c +++ b/crypto/bn/bn_err.c @@ -94,6 +94,7 @@ static ERR_STRING_DATA BN_str_functs[] = { {ERR_FUNC(BN_F_BN_GF2M_MOD_SOLVE_QUAD_ARR), "BN_GF2m_mod_solve_quad_arr"}, {ERR_FUNC(BN_F_BN_GF2M_MOD_SQR), "BN_GF2m_mod_sqr"}, {ERR_FUNC(BN_F_BN_GF2M_MOD_SQRT), "BN_GF2m_mod_sqrt"}, + {ERR_FUNC(BN_F_BN_LSHIFT), "BN_lshift"}, {ERR_FUNC(BN_F_BN_MOD_EXP2_MONT), "BN_mod_exp2_mont"}, {ERR_FUNC(BN_F_BN_MOD_EXP_MONT), "BN_mod_exp_mont"}, {ERR_FUNC(BN_F_BN_MOD_EXP_MONT_CONSTTIME), "BN_mod_exp_mont_consttime"}, @@ -109,6 +110,7 @@ static ERR_STRING_DATA BN_str_functs[] = { {ERR_FUNC(BN_F_BN_NEW), "BN_new"}, {ERR_FUNC(BN_F_BN_RAND), "BN_rand"}, {ERR_FUNC(BN_F_BN_RAND_RANGE), "BN_rand_range"}, + {ERR_FUNC(BN_F_BN_RSHIFT), "BN_rshift"}, {ERR_FUNC(BN_F_BN_USUB), "BN_usub"}, {0, NULL} }; @@ -125,6 +127,7 @@ static ERR_STRING_DATA BN_str_reasons[] = { {ERR_REASON(BN_R_INPUT_NOT_REDUCED), "input not reduced"}, {ERR_REASON(BN_R_INVALID_LENGTH), "invalid length"}, {ERR_REASON(BN_R_INVALID_RANGE), "invalid range"}, + {ERR_REASON(BN_R_INVALID_SHIFT), "invalid shift"}, {ERR_REASON(BN_R_NOT_A_SQUARE), "not a square"}, {ERR_REASON(BN_R_NOT_INITIALIZED), "not initialized"}, {ERR_REASON(BN_R_NO_INVERSE), "no inverse"}, diff --git a/crypto/bn/bn_shift.c b/crypto/bn/bn_shift.c index 4f3e8ffed7..9673d9a306 100644 --- a/crypto/bn/bn_shift.c +++ b/crypto/bn/bn_shift.c @@ -137,6 +137,11 @@ int BN_lshift(BIGNUM *r, const BIGNUM *a, int n) bn_check_top(r); bn_check_top(a); + if (n < 0) { + BNerr(BN_F_BN_LSHIFT, BN_R_INVALID_SHIFT); + return 0; + } + r->neg = a->neg; nw = n / BN_BITS2; if (bn_wexpand(r, a->top + nw + 1) == NULL) @@ -174,6 +179,11 @@ int BN_rshift(BIGNUM *r, const BIGNUM *a, int n) bn_check_top(r); bn_check_top(a); + if (n < 0) { + BNerr(BN_F_BN_RSHIFT, BN_R_INVALID_SHIFT); + return 0; + } + nw = n / BN_BITS2; rb = n % BN_BITS2; lb = BN_BITS2 - rb; diff --git a/doc/crypto/BN_set_bit.pod b/doc/crypto/BN_set_bit.pod index b7c47b9b01..a32cca2cee 100644 --- a/doc/crypto/BN_set_bit.pod +++ b/doc/crypto/BN_set_bit.pod @@ -37,12 +37,12 @@ BN_mask_bits() truncates B to an B bit number shorter than B bits. BN_lshift() shifts B left by B bits and places the result in -B (C). BN_lshift1() shifts B left by one and places -the result in B (C). +B (C). Note that B must be non-negative. BN_lshift1() shifts +B left by one and places the result in B (C). BN_rshift() shifts B right by B bits and places the result in -B (C). BN_rshift1() shifts B right by one and places -the result in B (C). +B (C). Note that B must be non-negative. BN_rshift1() shifts +B right by one and places the result in B (C). For the shift functions, B and B may be the same variable. -- 2.34.1