diff --git a/src/secp256k1/src/ecmult_const_impl.h b/src/secp256k1/src/ecmult_const_impl.h --- a/src/secp256k1/src/ecmult_const_impl.h +++ b/src/secp256k1/src/ecmult_const_impl.h @@ -105,16 +105,22 @@ /* 4 */ u_last = secp256k1_scalar_shr_int(&s, w); do { - int sign; int even; /* 4.1 4.4 */ u = secp256k1_scalar_shr_int(&s, w); /* 4.2 */ even = ((u & 1) == 0); - sign = 2 * (u_last > 0) - 1; - u += sign * even; - u_last -= sign * even * (1 << w); + /* In contrast to the original algorithm, u_last is always > 0 and + * therefore we do not need to check its sign. In particular, it's easy + * to see that u_last is never < 0 because u is never < 0. Moreover, + * u_last is never = 0 because u is never even after a loop + * iteration. The same holds analogously for the initial value of + * u_last (in the first loop iteration). */ + VERIFY_CHECK(u_last > 0); + VERIFY_CHECK((u_last & 1) == 1); + u += even; + u_last -= even * (1 << w); /* 4.3, adapted for global sign change */ wnaf[word++] = u_last * global_sign; diff --git a/src/secp256k1/src/tests.c b/src/secp256k1/src/tests.c --- a/src/secp256k1/src/tests.c +++ b/src/secp256k1/src/tests.c @@ -3231,6 +3231,7 @@ int skew; int bits = 256; secp256k1_scalar num = *number; + secp256k1_scalar scalar_skew; secp256k1_scalar_set_int(&x, 0); secp256k1_scalar_set_int(&shift, 1 << w); @@ -3261,7 +3262,8 @@ secp256k1_scalar_add(&x, &x, &t); } /* Skew num because when encoding numbers as odd we use an offset */ - secp256k1_scalar_cadd_bit(&num, skew == 2, 1); + secp256k1_scalar_set_int(&scalar_skew, 1 << (skew == 2)); + secp256k1_scalar_add(&num, &num, &scalar_skew); CHECK(secp256k1_scalar_eq(&x, &num)); } @@ -3373,13 +3375,32 @@ int i; secp256k1_scalar n = {{0}}; + test_constant_wnaf(&n, 4); /* Sanity check: 1 and 2 are the smallest odd and even numbers and should * have easier-to-diagnose failure modes */ n.d[0] = 1; test_constant_wnaf(&n, 4); n.d[0] = 2; test_constant_wnaf(&n, 4); - /* Test 0 */ + /* Test -1, because it's a special case in wnaf_const */ + n = secp256k1_scalar_one; + secp256k1_scalar_negate(&n, &n); + test_constant_wnaf(&n, 4); + + /* Test -2, which may not lead to overflows in wnaf_const */ + secp256k1_scalar_add(&n, &secp256k1_scalar_one, &secp256k1_scalar_one); + secp256k1_scalar_negate(&n, &n); + test_constant_wnaf(&n, 4); + + /* Test (1/2) - 1 = 1/-2 and 1/2 = (1/-2) + 1 + as corner cases of negation handling in wnaf_const */ + secp256k1_scalar_inverse(&n, &n); + test_constant_wnaf(&n, 4); + + secp256k1_scalar_add(&n, &n, &secp256k1_scalar_one); + test_constant_wnaf(&n, 4); + + /* Test 0 for fixed wnaf */ test_fixed_wnaf_small(); /* Random tests */ for (i = 0; i < count; i++) {