Skip to content

Commit

Permalink
Merge bitcoin#699: Initialize field elements when resulting in infinity
Browse files Browse the repository at this point in the history
47a7b83 Clear field elements when writing infinity (Elichai Turkel)
61d1ecb Added test with additions resulting in infinity (Elichai Turkel)

Pull request description:

  Currently if `secp256k1_gej_add_var` / `secp256k1_gej_add_ge_var` /` secp256k1_gej_add_zinv_var` receive `P + (-P)` it will set `gej->infinity = 1` but doesn't call initialize the field elements.
  Notice that this is the only branch in the function that results in an uninitialized output.

  By using `secp256k1_gej_set_infinity()` it will set the field elements to zero while also setting the infinity flag.

  I also added a test that fails with valgrind on current master but passes with the fix.

  EDIT: This isn't a bug or something necessary, I just personally found this helpful.

ACKs for top commit:
  real-or-random:
    ACK 47a7b83

Tree-SHA512: cdc2efc242a1b04b4f081183c07d4b2602cdba705e6b30b548df4e115e54fb97691f4b1a28f142f02d5e523c020721337a297b17d732acde147b910f5c53bd0a
  • Loading branch information
real-or-random committed Sep 9, 2020
2 parents 54caf2e + 47a7b83 commit 875d68b
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/group_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ static void secp256k1_gej_add_var(secp256k1_gej *r, const secp256k1_gej *a, cons
if (rzr != NULL) {
secp256k1_fe_set_int(rzr, 0);
}
r->infinity = 1;
secp256k1_gej_set_infinity(r);
}
return;
}
Expand Down Expand Up @@ -449,7 +449,7 @@ static void secp256k1_gej_add_ge_var(secp256k1_gej *r, const secp256k1_gej *a, c
if (rzr != NULL) {
secp256k1_fe_set_int(rzr, 0);
}
r->infinity = 1;
secp256k1_gej_set_infinity(r);
}
return;
}
Expand Down Expand Up @@ -508,7 +508,7 @@ static void secp256k1_gej_add_zinv_var(secp256k1_gej *r, const secp256k1_gej *a,
if (secp256k1_fe_normalizes_to_zero_var(&i)) {
secp256k1_gej_double_var(r, a, NULL);
} else {
r->infinity = 1;
secp256k1_gej_set_infinity(r);
}
return;
}
Expand Down
34 changes: 34 additions & 0 deletions src/tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -2318,6 +2318,39 @@ void test_ge(void) {
free(zinv);
}


void test_intialized_inf(void) {
secp256k1_ge p;
secp256k1_gej pj, npj, infj1, infj2, infj3;
secp256k1_fe zinv;

/* Test that adding P+(-P) results in a fully initalized infinity*/
random_group_element_test(&p);
secp256k1_gej_set_ge(&pj, &p);
secp256k1_gej_neg(&npj, &pj);

secp256k1_gej_add_var(&infj1, &pj, &npj, NULL);
CHECK(secp256k1_gej_is_infinity(&infj1));
CHECK(secp256k1_fe_is_zero(&infj1.x));
CHECK(secp256k1_fe_is_zero(&infj1.y));
CHECK(secp256k1_fe_is_zero(&infj1.z));

secp256k1_gej_add_ge_var(&infj2, &npj, &p, NULL);
CHECK(secp256k1_gej_is_infinity(&infj2));
CHECK(secp256k1_fe_is_zero(&infj2.x));
CHECK(secp256k1_fe_is_zero(&infj2.y));
CHECK(secp256k1_fe_is_zero(&infj2.z));

secp256k1_fe_set_int(&zinv, 1);
secp256k1_gej_add_zinv_var(&infj3, &npj, &p, &zinv);
CHECK(secp256k1_gej_is_infinity(&infj3));
CHECK(secp256k1_fe_is_zero(&infj3.x));
CHECK(secp256k1_fe_is_zero(&infj3.y));
CHECK(secp256k1_fe_is_zero(&infj3.z));


}

void test_add_neg_y_diff_x(void) {
/* The point of this test is to check that we can add two points
* whose y-coordinates are negatives of each other but whose x
Expand Down Expand Up @@ -2391,6 +2424,7 @@ void run_ge(void) {
test_ge();
}
test_add_neg_y_diff_x();
test_intialized_inf();
}

void test_ec_combine(void) {
Expand Down

0 comments on commit 875d68b

Please sign in to comment.