Skip to content

Some high "s" values in ECDSA signature not detected due to error in floating point arithmetic #353

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

Closed
salmonberry7 opened this issue Feb 3, 2025 · 1 comment · Fixed by #358
Labels
bug unintended behaviour in ecdsa code
Milestone

Comments

@salmonberry7
Copy link

salmonberry7 commented Feb 3, 2025

In the functions such as sigencode_strings_canonize, sigencode_string_canonize, and sigencode_der_canonize
where canonicalization of the "s" value in an ECDSA signature is performed with the code :

if s > order / 2:
	s = order - s

which uses the Python 3 "true division" operator / producing a floating point type irrespective of the type of the operands (eg. as described in [1] p146), high "s" values in at least the range [N // 2 + 1, N // 2 + 2**127] are not detected, where // is the Python 3 "floor division" operator which discards the remainder and performs exact integer arithmetic for integer operands of arbitrary size (eg. see [1] p135)). (N = order of the elliptic curve).

This is because the exact value of N / 2, which should be N // 2 + 0.5, is considerably larger than this due to the floating point error:

>>> N/2 > N//2 + 2**127
True
>>> N/2 > N//2 + 2**128
False

(testing with Python interpreter v3.8.10).

In the test below in Python interpreter v3.8.10 we can see how s = (N // 2) + 2**127 is not detected as high, whereas s = (N // 2) + 2**128 is detected as high :

N = order of secp256k1 generator point G =
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141

N // 2 = 
7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0        (exact)

>>> N / 2
5.78960446186581e+76            (approximate)

>>> s = (N // 2) + 2**127
>>> s > N / 2
False

>>> s = (N // 2) + 2**128
>>> s > N / 2
True

The fix is to simply use the exact integer floor division operator // instead of / :

if s > order // 2:
	s = order - s

and this will detect any s from the "midpoint" of odd prime N upwards, ie. the "high" values of s.

This problem is also discussed in this question on Bitcoin Stack Exchange.

[1] Mark Lutz (2013), Learning Python 5th Edition, O'Reilly

@tomato42
Copy link
Member

tomato42 commented Feb 4, 2025

Good find, yes, those changes look good. Could you prepare a PR fixing them?

@tomato42 tomato42 added this to the v0.20.0 milestone Feb 20, 2025
@tomato42 tomato42 added the bug unintended behaviour in ecdsa code label Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug unintended behaviour in ecdsa code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants