indutny / elliptic

Fast Elliptic Curve Cryptography in plain javascript
1.68k stars 373 forks source link

⚠️ BN.toString(16) can result in wrong output (bn.js@v5.2.0) #299

Open matthiasgeihs opened 1 year ago

matthiasgeihs commented 1 year ago

Citing https://github.com/indutny/bn.js/pull/295:

In some circumstances the hex encoding of big numbers is wrong. In addition to a display issue, given that the the hex string if often used as an intermediate representation in transport/conversion scenarios, the re-constructed big number can actually change its value, creating serious issues.

The issue has been fixed in bn.js@bn.js@v5.2.1. elliptic should update to this version.

nkavian commented 1 year ago

I'm almost certain this is the bug I ran into. Using elliptic through TronWeb, the signature has a correct "r" value, but the "s" and "v" value are wrong.

When performing the same exact signature using secp256k1 and server side Java bouncycastle; it works as expected.

I verified the "message" in all 3 cases is converted into the same exact Big Int so the "message" is not the source of any issues.

Also, it happened for a specific key we have. When trying a different private key, all 3 signatures matched.