Skip to content

Commit c727086

Browse files
peterdettmansipa
authored andcommitted
Optimization: use Nx32 representation for recoded bits
The existing code needs to deal with the edge case that bit_pos >= 256, which would lead to an out-of-bounds read from secp256k1_scalar. Instead, recode the scalar into an array of uint32_t with enough zero padding at the end to alleviate the issue. This also simplifies the code, and is necessary for a security improvement in a follow-up commit. Original code by Peter Dettman, with modifications by Pieter Wuille.
1 parent 2e8e8c3 commit c727086

File tree

1 file changed

+15
-6
lines changed

1 file changed

+15
-6
lines changed

src/ecmult_gen_impl.h

+15-6
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,12 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
5353
secp256k1_ge add;
5454
secp256k1_fe neg;
5555
secp256k1_ge_storage adds;
56-
secp256k1_scalar recoded;
57-
int first = 1;
56+
secp256k1_scalar tmp;
57+
/* Array of uint32_t values large enough to store COMB_BITS bits. Only the bottom
58+
* 8 are ever nonzero, but having the zero padding at the end if COMB_BITS>256
59+
* avoids the need to deal with out-of-bounds reads from a scalar. */
60+
uint32_t recoded[(COMB_BITS + 31) >> 5] = {0};
61+
int first = 1, i;
5862

5963
memset(&adds, 0, sizeof(adds));
6064

@@ -74,7 +78,12 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
7478
*/
7579

7680
/* Compute the scalar (gn + ctx->scalar_offset). */
77-
secp256k1_scalar_add(&recoded, &ctx->scalar_offset, gn);
81+
secp256k1_scalar_add(&tmp, &ctx->scalar_offset, gn);
82+
/* Convert to recoded array. */
83+
for (i = 0; i < 8; ++i) {
84+
recoded[i] = secp256k1_scalar_get_bits(&tmp, 32 * i, 32);
85+
}
86+
secp256k1_scalar_clear(&tmp);
7887

7988
/* In secp256k1_ecmult_gen_prec_table we have precomputed sums of the
8089
* (2*d_i-1) * 2^(i-1) * G points, for various combinations of i positions.
@@ -142,8 +151,8 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
142151
* together: bit (tooth) of bits = bit
143152
* ((block*COMB_TEETH + tooth)*COMB_SPACING + comb_off) of recoded. */
144153
uint32_t bits = 0, sign, abs, index, tooth;
145-
for (tooth = 0; tooth < COMB_TEETH && bit_pos < 256; ++tooth) {
146-
uint32_t bit = secp256k1_scalar_get_bits(&recoded, bit_pos, 1);
154+
for (tooth = 0; tooth < COMB_TEETH; ++tooth) {
155+
uint32_t bit = (recoded[bit_pos >> 5] >> (bit_pos & 0x1f)) & 1;
147156
bits |= bit << tooth;
148157
bit_pos += COMB_SPACING;
149158
}
@@ -198,7 +207,7 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
198207
secp256k1_fe_clear(&neg);
199208
secp256k1_ge_clear(&add);
200209
memset(&adds, 0, sizeof(adds));
201-
secp256k1_scalar_clear(&recoded);
210+
memset(&recoded, 0, sizeof(recoded));
202211
}
203212

204213
/* Setup blinding values for secp256k1_ecmult_gen. */

0 commit comments

Comments
 (0)