Skip to content

Commit

Permalink
Merge bitcoin-core/secp256k1#1316: Do not invoke fe_is_zero on failed…
Browse files Browse the repository at this point in the history
… set_b32_limit

6433175 Do not invoke fe_is_zero on failed set_b32_limit (Pieter Wuille)

Pull request description:

  Noticed in the CI output of bitcoin#1313 (https://cirrus-ci.com/task/5117786435878912)

  The code violates the field element contract that states that a field element that comes out of a failed `secp256k1_fe_set_b32_limit` call cannot be used before overwriting it. This is not an issue in practice, as such failure can only occur with negligible probability, but the experimental compiler in that CI setting is technically correct in detecting this possibility.

  Fix it by setting it to 1 based on a `secp256k1_fe_normalizes_to_zero` test rather than a `secp256k1_fe_is_zero` one (which does not require normalization).

ACKs for top commit:
  stratospher:
    ACK 6433175
  real-or-random:
    utACK 6433175

Tree-SHA512: 49da4535181c4607c1f4d23d1fd7cd65e7751c7cfa68643f1da77f3ec7961754fc8553bb415137fd61d86c805fe69f5adf97c05b9dc4d3bf357ae7c6409cc51a
  • Loading branch information
real-or-random committed May 23, 2023
2 parents 5f7903c + 6433175 commit d373a72
Showing 1 changed file with 2 additions and 5 deletions.
7 changes: 2 additions & 5 deletions src/ecmult_gen_impl.h
Expand Up @@ -87,7 +87,6 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const
secp256k1_fe s;
unsigned char nonce32[32];
secp256k1_rfc6979_hmac_sha256 rng;
int overflow;
unsigned char keydata[64];
if (seed32 == NULL) {
/* When seed is NULL, reset the initial point and blinding value. */
Expand All @@ -106,11 +105,9 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const
memcpy(keydata + 32, seed32, 32);
secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, 64);
memset(keydata, 0, sizeof(keydata));
/* Accept unobservably small non-uniformity. */
secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32);
overflow = !secp256k1_fe_set_b32_limit(&s, nonce32);
overflow |= secp256k1_fe_is_zero(&s);
secp256k1_fe_cmov(&s, &secp256k1_fe_one, overflow);
secp256k1_fe_set_b32_mod(&s, nonce32);
secp256k1_fe_cmov(&s, &secp256k1_fe_one, secp256k1_fe_normalizes_to_zero(&s));
/* Randomize the projection to defend against multiplier sidechannels.
Do this before our own call to secp256k1_ecmult_gen below. */
secp256k1_gej_rescale(&ctx->initial, &s);
Expand Down

0 comments on commit d373a72

Please sign in to comment.