From dd6c3de322740a3054cf6a1994a38dc8f201b473 Mon Sep 17 00:00:00 2001 From: Russell O'Connor Date: Tue, 4 May 2021 14:59:47 -0400 Subject: [PATCH 1/5] Have secp256k1_ge_set_gej_var initialize all fields. Previous behaviour would not initialize r->x and r->y values in the case where infinity is passed in. --- src/group_impl.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/group_impl.h b/src/group_impl.h index 19ebd8f44ee38..1b69082c21154 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -100,8 +100,8 @@ static void secp256k1_ge_set_gej(secp256k1_ge *r, secp256k1_gej *a) { static void secp256k1_ge_set_gej_var(secp256k1_ge *r, secp256k1_gej *a) { secp256k1_fe z2, z3; - r->infinity = a->infinity; if (a->infinity) { + secp256k1_ge_set_infinity(r); return; } secp256k1_fe_inv_var(&a->z, &a->z); @@ -110,8 +110,7 @@ static void secp256k1_ge_set_gej_var(secp256k1_ge *r, secp256k1_gej *a) { secp256k1_fe_mul(&a->x, &a->x, &z2); secp256k1_fe_mul(&a->y, &a->y, &z3); secp256k1_fe_set_int(&a->z, 1); - r->x = a->x; - r->y = a->y; + secp256k1_ge_set_xy(r, &a->x, &a->y); } static void secp256k1_ge_set_all_gej_var(secp256k1_ge *r, const secp256k1_gej *a, size_t len) { From 31c0f6de413e521731ad0e63424431b3dd49cec8 Mon Sep 17 00:00:00 2001 From: Russell O'Connor Date: Tue, 4 May 2021 15:49:48 -0400 Subject: [PATCH 2/5] Have secp256k1_gej_double_var initialize all fields. Previous behaviour would not initialize r->x and r->y values in the case where infinity is passed in. --- src/group_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/group_impl.h b/src/group_impl.h index 1b69082c21154..5ed45fda66998 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -310,7 +310,7 @@ static void secp256k1_gej_double_var(secp256k1_gej *r, const secp256k1_gej *a, s * point will be gibberish (z = 0 but infinity = 0). */ if (a->infinity) { - r->infinity = 1; + secp256k1_gej_set_infinity(r); if (rzr != NULL) { secp256k1_fe_set_int(rzr, 1); } From 45b6468d7e3ed9849ed474c71e9a9479de1a77db Mon Sep 17 00:00:00 2001 From: Russell O'Connor Date: Tue, 4 May 2021 16:17:00 -0400 Subject: [PATCH 3/5] Have secp256k1_ge_set_all_gej_var initialize all fields. Previous behaviour would not initialize r->y values in the case where infinity is passed in. Furthermore, the previous behaviour wouldn't initialize anything in the case where all inputs were infinity. --- src/group_impl.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/group_impl.h b/src/group_impl.h index 5ed45fda66998..47aea32be184a 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -119,7 +119,9 @@ static void secp256k1_ge_set_all_gej_var(secp256k1_ge *r, const secp256k1_gej *a size_t last_i = SIZE_MAX; for (i = 0; i < len; i++) { - if (!a[i].infinity) { + if (a[i].infinity) { + secp256k1_ge_set_infinity(&r[i]); + } else { /* Use destination's x coordinates as scratch space */ if (last_i == SIZE_MAX) { r[i].x = a[i].z; @@ -147,7 +149,6 @@ static void secp256k1_ge_set_all_gej_var(secp256k1_ge *r, const secp256k1_gej *a r[last_i].x = u; for (i = 0; i < len; i++) { - r[i].infinity = a[i].infinity; if (!a[i].infinity) { secp256k1_ge_set_gej_zinv(&r[i], &a[i], &r[i].x); } From 4a19668c37bc77d0165f4a1c0e626e321e9c4a09 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 5 May 2021 09:38:22 +0200 Subject: [PATCH 4/5] tests: Test secp256k1_ge_set_all_gej_var for all infinity inputs --- src/tests.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/tests.c b/src/tests.c index a146394305cb6..4078600012345 100644 --- a/src/tests.c +++ b/src/tests.c @@ -3115,6 +3115,17 @@ void test_ge(void) { ge_equals_gej(&ge[i], &gej[i]); } + /* Test batch gej -> ge conversion with all infinities. */ + for (i = 0; i < 4 * runs + 1; i++) { + secp256k1_gej_set_infinity(&gej[i]); + } + /* batch convert */ + secp256k1_ge_set_all_gej_var(ge, gej, 4 * runs + 1); + /* check result */ + for (i = 0; i < 4 * runs + 1; i++) { + CHECK(secp256k1_ge_is_infinity(&ge[i])); + } + free(ge); free(gej); } From 14c9739a1fb485bb56dbe3447132a37bcbef4e22 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 5 May 2021 09:38:22 +0200 Subject: [PATCH 5/5] tests: Improve secp256k1_ge_set_all_gej_var for some infinity inputs --- src/tests.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/tests.c b/src/tests.c index 4078600012345..2764bc6aa327c 100644 --- a/src/tests.c +++ b/src/tests.c @@ -3101,14 +3101,17 @@ void test_ge(void) { /* Test batch gej -> ge conversion with many infinities. */ for (i = 0; i < 4 * runs + 1; i++) { + int odd; random_group_element_test(&ge[i]); + odd = secp256k1_fe_is_odd(&ge[i].x); + CHECK(odd == 0 || odd == 1); /* randomly set half the points to infinity */ - if(secp256k1_fe_is_odd(&ge[i].x)) { + if (odd == i % 2) { secp256k1_ge_set_infinity(&ge[i]); } secp256k1_gej_set_ge(&gej[i], &ge[i]); } - /* batch invert */ + /* batch convert */ secp256k1_ge_set_all_gej_var(ge, gej, 4 * runs + 1); /* check result */ for (i = 0; i < 4 * runs + 1; i++) {