Skip to content

Fix missing clears on secret values #367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mllwchrry wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix missing clears on secret values #367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/modules/ecdsa_s2c/main_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ int secp256k1_ecdsa_anti_exfil_signer_commit(const secp256k1_context* ctx, secp2

secp256k1_ecmult_gen_ge(&ctx->ecmult_gen_ctx, &r, &k);
secp256k1_ecdsa_s2c_opening_save(opening, &r);
secp256k1_scalar_clear(&k);
secp256k1_memclear_explicit(nonce32, 32);
return 1;
}
Expand Down
3 changes: 3 additions & 0 deletions src/modules/generator/main_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ static int secp256k1_generator_generate_internal(const secp256k1_context* ctx, s
secp256k1_scalar_set_b32(&blind, blind32, &overflow);
ret = !overflow;
secp256k1_ecmult_gen_gej(&ctx->ecmult_gen_ctx, &accum, &blind);
secp256k1_scalar_clear(&blind);
}

secp256k1_sha256_initialize(&sha256);
Expand Down Expand Up @@ -356,6 +357,8 @@ int secp256k1_pedersen_blind_sum(const secp256k1_context* ctx, unsigned char *bl
for (i = 0; i < n; i++) {
secp256k1_scalar_set_b32(&x, blinds[i], &overflow);
if (overflow) {
secp256k1_scalar_clear(&acc);
secp256k1_scalar_clear(&x);
return 0;
}
if (i >= npositive) {
Expand Down
16 changes: 15 additions & 1 deletion src/modules/rangeproof/rangeproof_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,8 @@ SECP256K1_INLINE static int secp256k1_rangeproof_sign_impl(const secp256k1_hash_
prep[idx] = 128;
}
if (!secp256k1_rangeproof_genrand(hash_ctx, sec, s, prep, rsizes, rings, nonce, commit, proof, len, genp)) {
secp256k1_memclear_explicit(prep, sizeof(prep));
secp256k1_memclear_explicit(sec, sizeof(sec));
return 0;
}
secp256k1_memclear_explicit(prep, 4096);
Expand All @@ -284,7 +286,10 @@ SECP256K1_INLINE static int secp256k1_rangeproof_sign_impl(const secp256k1_hash_
*/
secp256k1_scalar_set_b32(&stmp, blind, &overflow);
secp256k1_scalar_add(&sec[rings - 1], &sec[rings - 1], &stmp);
secp256k1_scalar_clear(&stmp);
if (overflow || secp256k1_scalar_is_zero(&sec[rings - 1])) {
secp256k1_memclear_explicit(sec, sizeof(sec));
secp256k1_memclear_explicit(k, sizeof(k));
return 0;
}
signs = &proof[len];
Expand All @@ -298,6 +303,8 @@ SECP256K1_INLINE static int secp256k1_rangeproof_sign_impl(const secp256k1_hash_
/*OPT: Use the precomputed gen2 basis?*/
secp256k1_pedersen_ecmult(ecmult_gen_ctx, &pubs[npub], &sec[i], ((uint64_t)secidx[i] * scale) << (i*2), genp);
if (secp256k1_gej_is_infinity(&pubs[npub])) {
secp256k1_memclear_explicit(sec, sizeof(sec));
secp256k1_memclear_explicit(k, sizeof(k));
return 0;
}
if (i < rings - 1) {
Expand All @@ -323,6 +330,8 @@ SECP256K1_INLINE static int secp256k1_rangeproof_sign_impl(const secp256k1_hash_
secp256k1_sha256_finalize(hash_ctx, &sha256_m, tmp);
secp256k1_sha256_clear(&sha256_m);
if (!secp256k1_borromean_sign(hash_ctx, ecmult_gen_ctx, &proof[len], s, pubs, k, sec, rsizes, secidx, rings, tmp, 32)) {
secp256k1_memclear_explicit(sec, sizeof(sec));
secp256k1_memclear_explicit(k, sizeof(k));
return 0;
}
len += 32;
Expand All @@ -332,7 +341,8 @@ SECP256K1_INLINE static int secp256k1_rangeproof_sign_impl(const secp256k1_hash_
}
VERIFY_CHECK(len <= *plen);
*plen = len;
secp256k1_memclear_explicit(prep, 4096);
secp256k1_memclear_explicit(sec, sizeof(sec));
secp256k1_memclear_explicit(k, sizeof(k));
return 1;
}

Expand Down Expand Up @@ -658,18 +668,21 @@ SECP256K1_INLINE static int secp256k1_rangeproof_verify_impl(const secp256k1_has
return 0;
}
if (!secp256k1_rangeproof_rewind_inner(hash_ctx, &blind, &vv, message_out, outlen, evalues, s, rsizes, rings, nonce, commit, proof, offset_post_header, genp)) {
secp256k1_scalar_clear(&blind);
return 0;
}
/* Unwind apparently successful, see if the commitment can be reconstructed. */
/* FIXME: should check vv is in the mantissa's range. */
vv = (vv * scale) + *min_value;
secp256k1_pedersen_ecmult(ecmult_gen_ctx, &accj, &blind, vv, genp);
if (secp256k1_gej_is_infinity(&accj)) {
secp256k1_scalar_clear(&blind);
return 0;
}
secp256k1_gej_neg(&accj, &accj);
secp256k1_gej_add_ge_var(&accj, &accj, commit, NULL);
if (!secp256k1_gej_is_infinity(&accj)) {
secp256k1_scalar_clear(&blind);
return 0;
}
if (blindout) {
Expand All @@ -678,6 +691,7 @@ SECP256K1_INLINE static int secp256k1_rangeproof_verify_impl(const secp256k1_has
if (value_out) {
*value_out = vv;
}
secp256k1_scalar_clear(&blind);
}
return ret;
}
Expand Down
13 changes: 13 additions & 0 deletions src/modules/surjection/main_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,10 +304,13 @@ int secp256k1_surjectionproof_generate(const secp256k1_context* ctx, secp256k1_s
/* Compute secret key */
secp256k1_scalar_set_b32(&tmps, input_blinding_key, &overflow);
if (overflow) {
secp256k1_scalar_clear(&tmps);
return 0;
}
secp256k1_scalar_set_b32(&blinding_key, output_blinding_key, &overflow);
if (overflow) {
secp256k1_scalar_clear(&tmps);
secp256k1_scalar_clear(&blinding_key);
return 0;
}
/* If any input tag is equal to an output tag, verification will fail, because our ring
Expand All @@ -316,20 +319,25 @@ int secp256k1_surjectionproof_generate(const secp256k1_context* ctx, secp256k1_s
* this at the same time that we relax the max-256-inputs rule. */
for (i = 0; i < n_ephemeral_input_tags; i++) {
if (secp256k1_memcmp_var(ephemeral_input_tags[i].data, ephemeral_output_tag->data, sizeof(ephemeral_output_tag->data)) == 0) {
secp256k1_scalar_clear(&tmps);
secp256k1_scalar_clear(&blinding_key);
return 0;
}
}
secp256k1_scalar_negate(&tmps, &tmps);
secp256k1_scalar_add(&blinding_key, &blinding_key, &tmps);
secp256k1_scalar_clear(&tmps);

/* Compute public keys */
n_total_pubkeys = secp256k1_surjectionproof_n_total_inputs(ctx, proof);

if (n_used_pubkeys > n_total_pubkeys || n_total_pubkeys != n_ephemeral_input_tags) {
secp256k1_scalar_clear(&blinding_key);
return 0;
}

if (secp256k1_surjection_compute_public_keys(ring_pubkeys, n_used_pubkeys, ephemeral_input_tags, n_total_pubkeys, proof->used_inputs, ephemeral_output_tag, input_index, &ring_input_index) == 0) {
secp256k1_scalar_clear(&blinding_key);
return 0;
}

Expand All @@ -338,6 +346,7 @@ int secp256k1_surjectionproof_generate(const secp256k1_context* ctx, secp256k1_s
indices[0] = (int) ring_input_index;
secp256k1_surjection_genmessage(hash_ctx, msg32, ephemeral_input_tags, n_total_pubkeys, ephemeral_output_tag);
if (secp256k1_surjection_genrand(hash_ctx, borromean_s, n_used_pubkeys, &blinding_key) == 0) {
secp256k1_scalar_clear(&blinding_key);
return 0;
}
/* Borromean sign will overwrite one of the s values we just generated, so use
Expand All @@ -346,11 +355,15 @@ int secp256k1_surjectionproof_generate(const secp256k1_context* ctx, secp256k1_s
nonce = borromean_s[ring_input_index];
secp256k1_scalar_clear(&borromean_s[ring_input_index]);
if (secp256k1_borromean_sign(hash_ctx, &ctx->ecmult_gen_ctx, &proof->data[0], borromean_s, ring_pubkeys, &nonce, &blinding_key, rsizes, indices, 1, msg32, 32) == 0) {
secp256k1_scalar_clear(&blinding_key);
secp256k1_scalar_clear(&nonce);
return 0;
}
for (i = 0; i < n_used_pubkeys; i++) {
secp256k1_scalar_get_b32(&proof->data[32 + 32 * i], &borromean_s[i]);
}
secp256k1_scalar_clear(&blinding_key);
secp256k1_scalar_clear(&nonce);
return 1;
}

Expand Down
Loading