Skip to content

Commit b0210a9

Browse files
sipasipa
committed
Merge pull request bitcoin#135
ee3eb4b Fix a memory leak and add a number of small tests. (Gregory Maxwell)
2 parents 4d879a3 + ee3eb4b commit b0210a9

File tree

2 files changed

+72
-7
lines changed

2 files changed

+72
-7
lines changed

‎src/ecdsa_impl.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ static void secp256k1_ecdsa_stop(void) {
4848
if (secp256k1_ecdsa_consts == NULL)
4949
return;
5050

51-
secp256k1_ecdsa_consts_t *c = (secp256k1_ecdsa_consts_t*)secp256k1_ecmult_consts;
51+
secp256k1_ecdsa_consts_t *c = (secp256k1_ecdsa_consts_t*)secp256k1_ecdsa_consts;
5252
secp256k1_ecdsa_consts = NULL;
5353
free(c);
5454
}

‎src/tests.c‎

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ void run_scalar_tests(void) {
408408
}
409409

410410
{
411-
// (-1)+1 should be zero.
411+
/* (-1)+1 should be zero. */
412412
secp256k1_scalar_t s, o;
413413
secp256k1_scalar_set_int(&s, 1);
414414
secp256k1_scalar_negate(&o, &s);
@@ -418,7 +418,7 @@ void run_scalar_tests(void) {
418418

419419
#ifndef USE_NUM_NONE
420420
{
421-
// A scalar with value of the curve order should be 0.
421+
/* A scalar with value of the curve order should be 0. */
422422
secp256k1_num_t order;
423423
secp256k1_scalar_order_get_num(&order);
424424
unsigned char bin[32];
@@ -617,9 +617,17 @@ void gej_equals_gej(const secp256k1_gej_t *a, const secp256k1_gej_t *b) {
617617
}
618618

619619
void test_ge(void) {
620+
char ca[135];
621+
char cb[68];
622+
int rlen;
620623
secp256k1_ge_t a, b, i, n;
621624
random_group_element_test(&a);
622625
random_group_element_test(&b);
626+
rlen = sizeof(ca);
627+
secp256k1_ge_get_hex(ca,&rlen,&a);
628+
CHECK(rlen > 4 && rlen <= (int)sizeof(ca));
629+
rlen = sizeof(cb);
630+
secp256k1_ge_get_hex(cb,&rlen,&b); /* Intentionally undersized buffer. */
623631
n = a;
624632
secp256k1_fe_normalize(&a.y);
625633
secp256k1_fe_negate(&n.y, &a.y, 1);
@@ -771,6 +779,11 @@ void test_point_times_order(const secp256k1_gej_t *point) {
771779
secp256k1_ecmult(&res2, point, &nx, &nx); /* calc res2 = (order - x) * point + (order - x) * G; */
772780
secp256k1_gej_add_var(&res1, &res1, &res2);
773781
CHECK(secp256k1_gej_is_infinity(&res1));
782+
CHECK(secp256k1_gej_is_valid(&res1) == 0);
783+
secp256k1_ge_t res3;
784+
secp256k1_ge_set_gej(&res3, &res1);
785+
CHECK(secp256k1_ge_is_infinity(&res3));
786+
CHECK(secp256k1_ge_is_valid(&res3) == 0);
774787
}
775788

776789
void run_point_times_order(void) {
@@ -841,13 +854,17 @@ void random_sign(secp256k1_ecdsa_sig_t *sig, const secp256k1_scalar_t *key, cons
841854
}
842855

843856
void test_ecdsa_sign_verify(void) {
857+
int recid;
858+
int getrec;
844859
secp256k1_scalar_t msg, key;
845860
random_scalar_order_test(&msg);
846861
random_scalar_order_test(&key);
847862
secp256k1_gej_t pubj; secp256k1_ecmult_gen(&pubj, &key);
848863
secp256k1_ge_t pub; secp256k1_ge_set_gej(&pub, &pubj);
849864
secp256k1_ecdsa_sig_t sig;
850-
random_sign(&sig, &key, &msg, NULL);
865+
getrec = secp256k1_rand32()&1;
866+
random_sign(&sig, &key, &msg, getrec?&recid:NULL);
867+
if (getrec) CHECK(recid >= 0 && recid < 4);
851868
CHECK(secp256k1_ecdsa_sig_verify(&sig, &pub, &msg));
852869
secp256k1_scalar_t one;
853870
secp256k1_scalar_set_int(&one, 1);
@@ -997,9 +1014,9 @@ void test_ecdsa_edge_cases(void) {
9971014
unsigned char pubkeyb[33];
9981015
int pubkeyblen = 33;
9991016
for (int recid = 0; recid < 4; recid++) {
1000-
// (4,4) encoded in DER.
1017+
/* (4,4) encoded in DER. */
10011018
unsigned char sigbder[8] = {0x30, 0x06, 0x02, 0x01, 0x04, 0x02, 0x01, 0x04};
1002-
// (order + r,4) encoded in DER.
1019+
/* (order + r,4) encoded in DER. */
10031020
unsigned char sigbderlong[40] = {
10041021
0x30, 0x26, 0x02, 0x21, 0x00, 0xFF, 0xFF, 0xFF,
10051022
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
@@ -1013,7 +1030,7 @@ void test_ecdsa_edge_cases(void) {
10131030
unsigned char pubkey2b[33];
10141031
int pubkey2blen = 33;
10151032
CHECK(secp256k1_ecdsa_recover_compact(msg32, 32, sigb64, pubkey2b, &pubkey2blen, 1, recid2));
1016-
// Verifying with (order + r,4) should always fail.
1033+
/* Verifying with (order + r,4) should always fail. */
10171034
CHECK(secp256k1_ecdsa_verify(msg32, 32, sigbderlong, sizeof(sigbderlong), pubkey2b, pubkey2blen) != 1);
10181035
}
10191036
/* Damage signature. */
@@ -1035,6 +1052,36 @@ void test_ecdsa_edge_cases(void) {
10351052
secp256k1_scalar_t msg = sig.s;
10361053
CHECK(secp256k1_ecdsa_sig_verify(&sig, &key, &msg) == 0);
10371054
}
1055+
1056+
/* Test r/s equal to zero */
1057+
{
1058+
/* (1,1) encoded in DER. */
1059+
unsigned char sigcder[8] = {0x30, 0x06, 0x02, 0x01, 0x01, 0x02, 0x01, 0x01};
1060+
unsigned char sigc64[64] = {
1061+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
1062+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
1063+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
1064+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
1065+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
1066+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
1067+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
1068+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
1069+
};
1070+
unsigned char pubkeyc[65];
1071+
int pubkeyclen = 65;
1072+
CHECK(secp256k1_ecdsa_recover_compact(msg32, 32, sigc64, pubkeyc, &pubkeyclen, 0, 0) == 1);
1073+
CHECK(secp256k1_ecdsa_verify(msg32, 32, sigcder, sizeof(sigcder), pubkeyc, pubkeyclen) == 1);
1074+
sigcder[4] = 0;
1075+
sigc64[31] = 0;
1076+
CHECK(secp256k1_ecdsa_recover_compact(msg32, 32, sigc64, pubkeyb, &pubkeyblen, 1, 0) == 0);
1077+
CHECK(secp256k1_ecdsa_verify(msg32, 32, sigcder, sizeof(sigcder), pubkeyc, pubkeyclen) == 0);
1078+
sigcder[4] = 1;
1079+
sigcder[7] = 0;
1080+
sigc64[31] = 1;
1081+
sigc64[63] = 0;
1082+
CHECK(secp256k1_ecdsa_recover_compact(msg32, 32, sigc64, pubkeyb, &pubkeyblen, 1, 0) == 0);
1083+
CHECK(secp256k1_ecdsa_verify(msg32, 32, sigcder, sizeof(sigcder), pubkeyc, pubkeyclen) == 0);
1084+
}
10381085
}
10391086

10401087
void run_ecdsa_edge_cases(void) {
@@ -1118,6 +1165,15 @@ int main(int argc, char **argv) {
11181165
/* initialize */
11191166
secp256k1_start(SECP256K1_START_SIGN | SECP256K1_START_VERIFY);
11201167

1168+
/* initializing a second time shouldn't cause any harm or memory leaks. */
1169+
secp256k1_start(SECP256K1_START_SIGN | SECP256K1_START_VERIFY);
1170+
1171+
/* Likewise, re-running the internal init functions should be harmless. */
1172+
secp256k1_fe_start();
1173+
secp256k1_ge_start();
1174+
secp256k1_scalar_start();
1175+
secp256k1_ecdsa_start();
1176+
11211177
#ifndef USE_NUM_NONE
11221178
/* num tests */
11231179
run_num_smalltests();
@@ -1154,5 +1210,14 @@ int main(int argc, char **argv) {
11541210

11551211
/* shutdown */
11561212
secp256k1_stop();
1213+
1214+
/* shutting down twice shouldn't cause any double frees. */
1215+
secp256k1_stop();
1216+
1217+
/* Same for the internal shutdown functions. */
1218+
secp256k1_fe_stop();
1219+
secp256k1_ge_stop();
1220+
secp256k1_scalar_stop();
1221+
secp256k1_ecdsa_stop();
11571222
return 0;
11581223
}

0 commit comments

Comments
 (0)