Skip to content

Commit c69dea0

Browse files
gmaxwellgmaxwell
committed
Clear output in more cases for pubkey_combine, adds tests.
Also corrects an outdated comment and adds an additional secp256k1_ecdsa_signature_parse_compact test.
1 parent 269d422 commit c69dea0

File tree

3 files changed

+80
-6
lines changed

3 files changed

+80
-6
lines changed

‎include/secp256k1.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -562,12 +562,10 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_context_randomize(
562562
* Returns: 1: the sum of the public keys is valid.
563563
* 0: the sum of the public keys is not valid.
564564
* Args: ctx: pointer to a context object
565-
* Out: out: pointer to pubkey for placing the resulting public key
565+
* Out: out: pointer to a public key object for placing the resulting public key
566566
* (cannot be NULL)
567567
* In: ins: pointer to array of pointers to public keys (cannot be NULL)
568568
* n: the number of public keys to add together (must be at least 1)
569-
* Use secp256k1_ec_pubkey_compress and secp256k1_ec_pubkey_decompress if the
570-
* uncompressed format is needed.
571569
*/
572570
SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_combine(
573571
const secp256k1_context* ctx,

‎src/secp256k1.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,7 @@ int secp256k1_ec_pubkey_combine(const secp256k1_context* ctx, secp256k1_pubkey *
536536
secp256k1_ge Q;
537537

538538
ARG_CHECK(pubnonce != NULL);
539+
memset(pubnonce, 0, sizeof(*pubnonce));
539540
ARG_CHECK(n >= 1);
540541
ARG_CHECK(pubnonces != NULL);
541542

@@ -546,7 +547,6 @@ int secp256k1_ec_pubkey_combine(const secp256k1_context* ctx, secp256k1_pubkey *
546547
secp256k1_gej_add_ge(&Qj, &Qj, &Q);
547548
}
548549
if (secp256k1_gej_is_infinity(&Qj)) {
549-
memset(pubnonce, 0, sizeof(*pubnonce));
550550
return 0;
551551
}
552552
secp256k1_ge_set_gej(&Q, &Qj);

‎src/tests.c‎

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2902,10 +2902,14 @@ void run_eckey_edge_case_test(void) {
29022902
0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41
29032903
};
29042904
const unsigned char zeros[sizeof(secp256k1_pubkey)] = {0x00};
2905-
unsigned char ctmp[32];
2906-
unsigned char ctmp2[32];
2905+
unsigned char ctmp[33];
2906+
unsigned char ctmp2[33];
29072907
secp256k1_pubkey pubkey;
29082908
secp256k1_pubkey pubkey2;
2909+
secp256k1_pubkey pubkey_one;
2910+
secp256k1_pubkey pubkey_negone;
2911+
const secp256k1_pubkey *pubkeys[3];
2912+
size_t len;
29092913
int32_t ecount;
29102914
/* Group order is too large, reject. */
29112915
CHECK(secp256k1_ec_seckey_verify(ctx, orderc) == 0);
@@ -2937,6 +2941,7 @@ void run_eckey_edge_case_test(void) {
29372941
CHECK(secp256k1_ec_pubkey_create(ctx, &pubkey, ctmp) == 1);
29382942
VG_CHECK(&pubkey, sizeof(pubkey));
29392943
CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0);
2944+
pubkey_one = pubkey;
29402945
/* Group order + 1 is too large, reject. */
29412946
memcpy(ctmp, orderc, 32);
29422947
ctmp[31] = 0x42;
@@ -2954,6 +2959,7 @@ void run_eckey_edge_case_test(void) {
29542959
CHECK(secp256k1_ec_pubkey_create(ctx, &pubkey, ctmp) == 1);
29552960
VG_CHECK(&pubkey, sizeof(pubkey));
29562961
CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0);
2962+
pubkey_negone = pubkey;
29572963
/* Tweak of zero leaves the value changed. */
29582964
memset(ctmp2, 0, 32);
29592965
CHECK(secp256k1_ec_privkey_tweak_add(ctx, ctmp, ctmp2) == 1);
@@ -3060,6 +3066,73 @@ void run_eckey_edge_case_test(void) {
30603066
CHECK(secp256k1_ec_pubkey_create(ctx, &pubkey, NULL) == 0);
30613067
CHECK(ecount == 2);
30623068
CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0);
3069+
/* secp256k1_ec_pubkey_combine tests. */
3070+
ecount = 0;
3071+
pubkeys[0] = &pubkey_one;
3072+
VG_UNDEF(&pubkeys[0], sizeof(secp256k1_pubkey *));
3073+
VG_UNDEF(&pubkeys[1], sizeof(secp256k1_pubkey *));
3074+
VG_UNDEF(&pubkeys[2], sizeof(secp256k1_pubkey *));
3075+
memset(&pubkey, 255, sizeof(secp256k1_pubkey));
3076+
VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
3077+
CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 0) == 0);
3078+
VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
3079+
CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0);
3080+
CHECK(ecount == 1);
3081+
memset(&pubkey, 255, sizeof(secp256k1_pubkey));
3082+
VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
3083+
CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, -1) == 0);
3084+
VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
3085+
CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0);
3086+
CHECK(ecount == 2);
3087+
CHECK(secp256k1_ec_pubkey_combine(ctx, NULL, pubkeys, 1) == 0);
3088+
CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0);
3089+
CHECK(ecount == 3);
3090+
memset(&pubkey, 255, sizeof(secp256k1_pubkey));
3091+
VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
3092+
CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, NULL, 1) == 0);
3093+
VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
3094+
CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0);
3095+
CHECK(ecount == 4);
3096+
pubkeys[0] = &pubkey_negone;
3097+
memset(&pubkey, 255, sizeof(secp256k1_pubkey));
3098+
VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
3099+
CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 1) == 1);
3100+
VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
3101+
CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0);
3102+
CHECK(ecount == 4);
3103+
len = 33;
3104+
CHECK(secp256k1_ec_pubkey_serialize(ctx, ctmp, &len, &pubkey, SECP256K1_EC_COMPRESSED) == 1);
3105+
CHECK(secp256k1_ec_pubkey_serialize(ctx, ctmp2, &len, &pubkey_negone, SECP256K1_EC_COMPRESSED) == 1);
3106+
CHECK(memcmp(ctmp, ctmp2, 33) == 0);
3107+
/* Result is infinity. */
3108+
pubkeys[0] = &pubkey_one;
3109+
pubkeys[1] = &pubkey_negone;
3110+
memset(&pubkey, 255, sizeof(secp256k1_pubkey));
3111+
VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
3112+
CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 2) == 0);
3113+
VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
3114+
CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0);
3115+
CHECK(ecount == 4);
3116+
/* Passes through infinity but comes out one. */
3117+
pubkeys[2] = &pubkey_one;
3118+
memset(&pubkey, 255, sizeof(secp256k1_pubkey));
3119+
VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
3120+
CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 3) == 1);
3121+
VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
3122+
CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0);
3123+
CHECK(ecount == 4);
3124+
len = 33;
3125+
CHECK(secp256k1_ec_pubkey_serialize(ctx, ctmp, &len, &pubkey, SECP256K1_EC_COMPRESSED) == 1);
3126+
CHECK(secp256k1_ec_pubkey_serialize(ctx, ctmp2, &len, &pubkey_one, SECP256K1_EC_COMPRESSED) == 1);
3127+
CHECK(memcmp(ctmp, ctmp2, 33) == 0);
3128+
/* Adds to two. */
3129+
pubkeys[1] = &pubkey_one;
3130+
memset(&pubkey, 255, sizeof(secp256k1_pubkey));
3131+
VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
3132+
CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 2) == 1);
3133+
VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
3134+
CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0);
3135+
CHECK(ecount == 4);
30633136
secp256k1_context_set_illegal_callback(ctx, NULL, NULL);
30643137
}
30653138

@@ -3933,6 +4006,9 @@ void test_ecdsa_edge_cases(void) {
39334006
CHECK(ecount == 5);
39344007
CHECK(secp256k1_ecdsa_signature_parse_compact(ctx, &sig, signature) == 1);
39354008
CHECK(ecount == 5);
4009+
memset(signature, 255, 64);
4010+
CHECK(secp256k1_ecdsa_signature_parse_compact(ctx, &sig, signature) == 0);
4011+
CHECK(ecount == 5);
39364012
secp256k1_context_set_illegal_callback(ctx, NULL, NULL);
39374013
}
39384014

0 commit comments

Comments
 (0)