From cd473e02c372217c3a6608ce5afaa543ed78f891 Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Sun, 26 May 2019 10:22:38 +0000 Subject: [PATCH] Avoid calling secp256k1_*_is_zero when secp256k1_*_set_b32 fails. Most of the codebase correctly used short-cutting to avoid calling _is_zero on possibly incompletely initialized elements, but a few places were missed. --- src/ecmult_gen_impl.h | 4 ++-- src/modules/recovery/main_impl.h | 2 +- src/secp256k1.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index f818d45c298b4..0e2eafa71de30 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -187,7 +187,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const do { secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32); retry = !secp256k1_fe_set_b32(&s, nonce32); - retry |= secp256k1_fe_is_zero(&s); + retry = retry || secp256k1_fe_is_zero(&s); } while (retry); /* This branch true is cryptographically unreachable. Requires sha256_hmac output > Fp. */ /* Randomize the projection to defend against multiplier sidechannels. */ secp256k1_gej_rescale(&ctx->initial, &s); @@ -196,7 +196,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32); secp256k1_scalar_set_b32(&b, nonce32, &retry); /* A blinding value of 0 works, but would undermine the projection hardening. */ - retry |= secp256k1_scalar_is_zero(&b); + retry = retry || secp256k1_scalar_is_zero(&b); } while (retry); /* This branch true is cryptographically unreachable. Requires sha256_hmac output > order. */ secp256k1_rfc6979_hmac_sha256_finalize(&rng); memset(nonce32, 0, 32); diff --git a/src/modules/recovery/main_impl.h b/src/modules/recovery/main_impl.h index 2f6691c5a1309..ed356e53a5c8f 100755 --- a/src/modules/recovery/main_impl.h +++ b/src/modules/recovery/main_impl.h @@ -147,7 +147,7 @@ int secp256k1_ecdsa_sign_recoverable(const secp256k1_context* ctx, secp256k1_ecd break; } secp256k1_scalar_set_b32(&non, nonce32, &overflow); - if (!secp256k1_scalar_is_zero(&non) && !overflow) { + if (!overflow && !secp256k1_scalar_is_zero(&non)) { if (secp256k1_ecdsa_sig_sign(&ctx->ecmult_gen_ctx, &r, &s, &sec, &msg, &non, &recid)) { break; } diff --git a/src/secp256k1.c b/src/secp256k1.c index 627ebe75e668d..63b7b193e32ed 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -507,7 +507,7 @@ int secp256k1_ec_pubkey_create(const secp256k1_context* ctx, secp256k1_pubkey *p ARG_CHECK(seckey != NULL); secp256k1_scalar_set_b32(&sec, seckey, &overflow); - ret = (!overflow) & (!secp256k1_scalar_is_zero(&sec)); + ret = !overflow && !secp256k1_scalar_is_zero(&sec); if (ret) { secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &pj, &sec); secp256k1_ge_set_gej(&p, &pj);