Repository: bitcoin-core/secp256k1
Pull request #699 (2019-12-10 16:13:20)

Initialize field elements when resulting in infinity
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.


Patch:
diff --git a/src/group_impl.h b/src/group_impl.h
index 9b93c39..ade7a59 100644
--- a/src/group_impl.h
+++ b/src/group_impl.h
@@ -397,7 +397,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;
     }
@@ -447,7 +447,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;
     }
@@ -506,7 +506,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;
     }


Test patch:
diff --git a/src/tests.c b/src/tests.c
index d1cfcb5..15012e8 100644
--- a/src/tests.c
+++ b/src/tests.c
@@ -2258,6 +2258,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
@@ -2331,6 +2364,7 @@ void run_ge(void) {
         test_ge();
     }
     test_add_neg_y_diff_x();
+    test_intialized_inf();
 }
 
 void test_ec_combine(void) {