Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug of leading zeros public keys #601

Closed
Telariust opened this issue Mar 18, 2019 · 2 comments
Closed

bug of leading zeros public keys #601

Telariust opened this issue Mar 18, 2019 · 2 comments

Comments

@Telariust
Copy link

I got a problem when I wrote code for sequentially calculating public keys (for the vanity generator). Verification fails when I tried to verify a bundle (created by a fast algorithm) using another function of the standard algorithm. I am attaching a simple demo C code and its output so that you can reproduce the problem.

If you look at the output, you will see that all coordinates have leading zeros. This is very similar to the sequel of "bug of leading zeros privkey" bitpay / bitcore-lib # 47 (github.com/iancoleman/bip39/issues/58).
Only now - for public keys.
If I'm right, we have a big hidden problem for a long time.
About once every 10,000, a public key with leading zeros occurs and it has a chance to calculate it incorrectly.
Only secp256k1_gej_add_ge() yields correct results.
Incorrect calculations for secp256k1_ecmult(), secp256k1_gej_add_ge_var(), secp256k1_gej_add_var().
I check the "correctness" of the result on the brainwalletx.github.io, (but there are no guarantees that it is also not verified by this bug).

Debian 4.13.0 amd64 SMP x86_64
I use the library from 02.2019 (tried 2018, 2017 too).
Library compiled as standard
./configure
or deployed for my system as
./configure --enable-ecmult-static-precomputation --with-bignum=gmp --with-scalar=64bit --with-field=64bit --with-asm=x86_64
Experiments have revealed that the situation is very acute if
./configure --with-field=32bit
There is clearly something broken - public keys with leading zeros lose half of the digits.
The rest of the keys do not affect.

Dear developer(s), please fix it!
compare.c.txt
compare_output_tests.log

@real-or-random
Copy link
Contributor

This smells like the same (non-)issue as in #565. Please have a look.

@Telariust
Copy link
Author

Ok! I read in /src/field.h
/** Compare two field elements. Requires both inputs to be normalized */
static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b);
and I added normalize_var before fe_cmp_var
secp256k1_fe_normalize_var(&packGe1[i].x);
secp256k1_fe_normalize_var(&packGe2[i].x);
if(1 && (0 != secp256k1_fe_cmp_var(&packGe1[i].x, &packGe2[i].x))
..............
secp256k1_fe_normalize_var(&packGe1[i].y);
secp256k1_fe_normalize_var(&packGe2[i].y);
if(1 && (0 != secp256k1_fe_cmp_var(&packGe1[i].y, &packGe2[i].y))
and verify success for all combinations!
"I learned something new today"
Big thx, master!) and close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants