From 93ee03aa337ee2324d31d1af210557b67994fdcf Mon Sep 17 00:00:00 2001 From: tb Date: Sun, 26 Dec 2021 15:16:50 +0000 Subject: [PATCH] Consistently call BN_init() before BN_with_flags() BN_with_flags() preserves the BN_FLG_MALLOCED flag of the destination which results in a potential use of an uninitialized bit. In practice this doesn't matter since we don't free the cloned BIGNUMs anyway. As jsing points out, these are mostly pointless noise and should be garbage collected. I'll leave that for another rainy day. Coverity flagged one instance BN_gcd_no_branch(), the rest was found by the ever so helpful grep(1). CID 345122 ok jsing --- lib/libcrypto/bn/bn_gcd.c | 40 +++++++++++++++++++++++++------------ lib/libcrypto/rsa/rsa_eay.c | 3 ++- lib/libcrypto/rsa/rsa_gen.c | 5 ++++- 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/lib/libcrypto/bn/bn_gcd.c b/lib/libcrypto/bn/bn_gcd.c index 469ae752fba..d756398c8f5 100644 --- a/lib/libcrypto/bn/bn_gcd.c +++ b/lib/libcrypto/bn/bn_gcd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: bn_gcd.c,v 1.15 2017/01/29 17:49:22 beck Exp $ */ +/* $OpenBSD: bn_gcd.c,v 1.16 2021/12/26 15:16:50 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -576,6 +576,9 @@ BN_mod_inverse_no_branch(BIGNUM *in, const BIGNUM *a, const BIGNUM *n, bn_check_top(a); bn_check_top(n); + BN_init(&local_A); + BN_init(&local_B); + BN_CTX_start(ctx); if ((A = BN_CTX_get(ctx)) == NULL) goto err; @@ -608,10 +611,12 @@ BN_mod_inverse_no_branch(BIGNUM *in, const BIGNUM *a, const BIGNUM *n, A->neg = 0; if (B->neg || (BN_ucmp(B, A) >= 0)) { - /* Turn BN_FLG_CONSTTIME flag on, so that when BN_div is invoked, - * BN_div_no_branch will be called eventually. - */ + /* + * Turn BN_FLG_CONSTTIME flag on, so that when BN_div is invoked, + * BN_div_no_branch will be called eventually. + */ pB = &local_B; + /* BN_init() done at the top of the function. */ BN_with_flags(pB, B, BN_FLG_CONSTTIME); if (!BN_nnmod(B, pB, A, ctx)) goto err; @@ -633,10 +638,12 @@ BN_mod_inverse_no_branch(BIGNUM *in, const BIGNUM *a, const BIGNUM *n, * sign*Y*a == A (mod |n|) */ - /* Turn BN_FLG_CONSTTIME flag on, so that when BN_div is invoked, - * BN_div_no_branch will be called eventually. - */ + /* + * Turn BN_FLG_CONSTTIME flag on, so that when BN_div is invoked, + * BN_div_no_branch will be called eventually. + */ pA = &local_A; + /* BN_init() done at the top of the function. */ BN_with_flags(pA, A, BN_FLG_CONSTTIME); /* (D, M) := (A/B, A%B) ... */ @@ -740,6 +747,9 @@ BN_gcd_no_branch(BIGNUM *in, const BIGNUM *a, const BIGNUM *n, goto err; R = in; + BN_init(&local_A); + BN_init(&local_B); + bn_check_top(a); bn_check_top(n); @@ -768,10 +778,12 @@ BN_gcd_no_branch(BIGNUM *in, const BIGNUM *a, const BIGNUM *n, A->neg = 0; if (B->neg || (BN_ucmp(B, A) >= 0)) { - /* Turn BN_FLG_CONSTTIME flag on, so that when BN_div is invoked, - * BN_div_no_branch will be called eventually. - */ + /* + * Turn BN_FLG_CONSTTIME flag on, so that when BN_div is invoked, + * BN_div_no_branch will be called eventually. + */ pB = &local_B; + /* BN_init() done at the top of the function. */ BN_with_flags(pB, B, BN_FLG_CONSTTIME); if (!BN_nnmod(B, pB, A, ctx)) goto err; @@ -793,10 +805,12 @@ BN_gcd_no_branch(BIGNUM *in, const BIGNUM *a, const BIGNUM *n, * sign*Y*a == A (mod |n|) */ - /* Turn BN_FLG_CONSTTIME flag on, so that when BN_div is invoked, - * BN_div_no_branch will be called eventually. - */ + /* + * Turn BN_FLG_CONSTTIME flag on, so that when BN_div is invoked, + * BN_div_no_branch will be called eventually. + */ pA = &local_A; + /* BN_init() done at the top of the function. */ BN_with_flags(pA, A, BN_FLG_CONSTTIME); /* (D, M) := (A/B, A%B) ... */ diff --git a/lib/libcrypto/rsa/rsa_eay.c b/lib/libcrypto/rsa/rsa_eay.c index 33201a8a8b7..e9fc67349b6 100644 --- a/lib/libcrypto/rsa/rsa_eay.c +++ b/lib/libcrypto/rsa/rsa_eay.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rsa_eay.c,v 1.51 2019/11/02 13:52:31 jsing Exp $ */ +/* $OpenBSD: rsa_eay.c,v 1.52 2021/12/26 15:16:50 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -753,6 +753,7 @@ RSA_eay_mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa, BN_CTX *ctx) goto err; /* compute I mod p */ + BN_init(&c); BN_with_flags(&c, I, BN_FLG_CONSTTIME); if (!BN_mod_ct(r1, &c, rsa->p, ctx)) diff --git a/lib/libcrypto/rsa/rsa_gen.c b/lib/libcrypto/rsa/rsa_gen.c index 596eb8eb783..1c37d8ef219 100644 --- a/lib/libcrypto/rsa/rsa_gen.c +++ b/lib/libcrypto/rsa/rsa_gen.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rsa_gen.c,v 1.22 2017/01/29 17:49:23 beck Exp $ */ +/* $OpenBSD: rsa_gen.c,v 1.23 2021/12/26 15:16:50 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -194,12 +194,14 @@ rsa_builtin_keygen(RSA *rsa, int bits, BIGNUM *e_value, BN_GENCB *cb) if (!BN_mul(r0, r1, r2, ctx)) /* (p-1)(q-1) */ goto err; + BN_init(&pr0); BN_with_flags(&pr0, r0, BN_FLG_CONSTTIME); if (!BN_mod_inverse_ct(rsa->d, rsa->e, &pr0, ctx)) /* d */ goto err; /* set up d for correct BN_FLG_CONSTTIME flag */ + BN_init(&d); BN_with_flags(&d, rsa->d, BN_FLG_CONSTTIME); /* calculate d mod (p-1) */ @@ -211,6 +213,7 @@ rsa_builtin_keygen(RSA *rsa, int bits, BIGNUM *e_value, BN_GENCB *cb) goto err; /* calculate inverse of q mod p */ + BN_init(&p); BN_with_flags(&p, rsa->p, BN_FLG_CONSTTIME); if (!BN_mod_inverse_ct(rsa->iqmp, rsa->q, &p, ctx)) goto err; -- 2.20.1