diff --git a/include/secp256k1.h b/include/secp256k1.h index 384cb81a350..e50b22bcf09 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -562,12 +562,10 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_context_randomize( * Returns: 1: the sum of the public keys is valid. * 0: the sum of the public keys is not valid. * Args: ctx: pointer to a context object - * Out: out: pointer to pubkey for placing the resulting public key + * Out: out: pointer to a public key object for placing the resulting public key * (cannot be NULL) * In: ins: pointer to array of pointers to public keys (cannot be NULL) * n: the number of public keys to add together (must be at least 1) - * Use secp256k1_ec_pubkey_compress and secp256k1_ec_pubkey_decompress if the - * uncompressed format is needed. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_combine( const secp256k1_context* ctx, diff --git a/src/secp256k1.c b/src/secp256k1.c index e06f7d942e5..53bef86fff2 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -536,6 +536,7 @@ int secp256k1_ec_pubkey_combine(const secp256k1_context* ctx, secp256k1_pubkey * secp256k1_ge Q; ARG_CHECK(pubnonce != NULL); + memset(pubnonce, 0, sizeof(*pubnonce)); ARG_CHECK(n >= 1); ARG_CHECK(pubnonces != NULL); @@ -546,7 +547,6 @@ int secp256k1_ec_pubkey_combine(const secp256k1_context* ctx, secp256k1_pubkey * secp256k1_gej_add_ge(&Qj, &Qj, &Q); } if (secp256k1_gej_is_infinity(&Qj)) { - memset(pubnonce, 0, sizeof(*pubnonce)); return 0; } secp256k1_ge_set_gej(&Q, &Qj); diff --git a/src/tests.c b/src/tests.c index be3c79e948d..af143c1ae84 100644 --- a/src/tests.c +++ b/src/tests.c @@ -2902,10 +2902,14 @@ void run_eckey_edge_case_test(void) { 0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41 }; const unsigned char zeros[sizeof(secp256k1_pubkey)] = {0x00}; - unsigned char ctmp[32]; - unsigned char ctmp2[32]; + unsigned char ctmp[33]; + unsigned char ctmp2[33]; secp256k1_pubkey pubkey; secp256k1_pubkey pubkey2; + secp256k1_pubkey pubkey_one; + secp256k1_pubkey pubkey_negone; + const secp256k1_pubkey *pubkeys[3]; + size_t len; int32_t ecount; /* Group order is too large, reject. */ CHECK(secp256k1_ec_seckey_verify(ctx, orderc) == 0); @@ -2937,6 +2941,7 @@ void run_eckey_edge_case_test(void) { CHECK(secp256k1_ec_pubkey_create(ctx, &pubkey, ctmp) == 1); VG_CHECK(&pubkey, sizeof(pubkey)); CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0); + pubkey_one = pubkey; /* Group order + 1 is too large, reject. */ memcpy(ctmp, orderc, 32); ctmp[31] = 0x42; @@ -2954,6 +2959,7 @@ void run_eckey_edge_case_test(void) { CHECK(secp256k1_ec_pubkey_create(ctx, &pubkey, ctmp) == 1); VG_CHECK(&pubkey, sizeof(pubkey)); CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0); + pubkey_negone = pubkey; /* Tweak of zero leaves the value changed. */ memset(ctmp2, 0, 32); CHECK(secp256k1_ec_privkey_tweak_add(ctx, ctmp, ctmp2) == 1); @@ -3060,6 +3066,73 @@ void run_eckey_edge_case_test(void) { CHECK(secp256k1_ec_pubkey_create(ctx, &pubkey, NULL) == 0); CHECK(ecount == 2); CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0); + /* secp256k1_ec_pubkey_combine tests. */ + ecount = 0; + pubkeys[0] = &pubkey_one; + VG_UNDEF(&pubkeys[0], sizeof(secp256k1_pubkey *)); + VG_UNDEF(&pubkeys[1], sizeof(secp256k1_pubkey *)); + VG_UNDEF(&pubkeys[2], sizeof(secp256k1_pubkey *)); + memset(&pubkey, 255, sizeof(secp256k1_pubkey)); + VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 0) == 0); + VG_CHECK(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0); + CHECK(ecount == 1); + memset(&pubkey, 255, sizeof(secp256k1_pubkey)); + VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, -1) == 0); + VG_CHECK(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0); + CHECK(ecount == 2); + CHECK(secp256k1_ec_pubkey_combine(ctx, NULL, pubkeys, 1) == 0); + CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0); + CHECK(ecount == 3); + memset(&pubkey, 255, sizeof(secp256k1_pubkey)); + VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, NULL, 1) == 0); + VG_CHECK(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0); + CHECK(ecount == 4); + pubkeys[0] = &pubkey_negone; + memset(&pubkey, 255, sizeof(secp256k1_pubkey)); + VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 1) == 1); + VG_CHECK(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0); + CHECK(ecount == 4); + len = 33; + CHECK(secp256k1_ec_pubkey_serialize(ctx, ctmp, &len, &pubkey, SECP256K1_EC_COMPRESSED) == 1); + CHECK(secp256k1_ec_pubkey_serialize(ctx, ctmp2, &len, &pubkey_negone, SECP256K1_EC_COMPRESSED) == 1); + CHECK(memcmp(ctmp, ctmp2, 33) == 0); + /* Result is infinity. */ + pubkeys[0] = &pubkey_one; + pubkeys[1] = &pubkey_negone; + memset(&pubkey, 255, sizeof(secp256k1_pubkey)); + VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 2) == 0); + VG_CHECK(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0); + CHECK(ecount == 4); + /* Passes through infinity but comes out one. */ + pubkeys[2] = &pubkey_one; + memset(&pubkey, 255, sizeof(secp256k1_pubkey)); + VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 3) == 1); + VG_CHECK(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0); + CHECK(ecount == 4); + len = 33; + CHECK(secp256k1_ec_pubkey_serialize(ctx, ctmp, &len, &pubkey, SECP256K1_EC_COMPRESSED) == 1); + CHECK(secp256k1_ec_pubkey_serialize(ctx, ctmp2, &len, &pubkey_one, SECP256K1_EC_COMPRESSED) == 1); + CHECK(memcmp(ctmp, ctmp2, 33) == 0); + /* Adds to two. */ + pubkeys[1] = &pubkey_one; + memset(&pubkey, 255, sizeof(secp256k1_pubkey)); + VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 2) == 1); + VG_CHECK(&pubkey, sizeof(secp256k1_pubkey)); + CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0); + CHECK(ecount == 4); secp256k1_context_set_illegal_callback(ctx, NULL, NULL); } @@ -3933,6 +4006,9 @@ void test_ecdsa_edge_cases(void) { CHECK(ecount == 5); CHECK(secp256k1_ecdsa_signature_parse_compact(ctx, &sig, signature) == 1); CHECK(ecount == 5); + memset(signature, 255, 64); + CHECK(secp256k1_ecdsa_signature_parse_compact(ctx, &sig, signature) == 0); + CHECK(ecount == 5); secp256k1_context_set_illegal_callback(ctx, NULL, NULL); }