Skip to content

fix: sync upstream secp256k1#177

Open
eomti-wm wants to merge 9 commits intodevfrom
fix/sync-upstream-secp256k1
Open

fix: sync upstream secp256k1#177
eomti-wm wants to merge 9 commits intodevfrom
fix/sync-upstream-secp256k1

Conversation

@eomti-wm
Copy link
Copy Markdown

Summary

This PR implements security hardening specifically for the secp256k1 curve validation and updates the internal libsecp256k1 library for better performance. These changes align our secp256k1 implementation with the latest security standards from upstream go-ethereum.

Key Changes

secp256k1 Security Hardening

  • Invalid-Curve Attack Prevention: Added strict coordinate range checks (x < P, y < P) in IsOnCurve for both CGO and nocgo paths within the secp256k1 package.
  • Entry Point Validation: Updated UnmarshalPubkey to use IsOnCurve for early rejection of invalid public key points.
  • EllipticCurve Interface: Introduced an abstraction to ensure consistent validation logic across different secp256k1 implementation paths.

References (go-ethereum upstream)

lightclient and others added 9 commits March 12, 2026 15:23
Co-Authored-By: Gary Rong <garyrong0905@gmail.com>
Fix ECIES invalid-curve handling in RLPx handshake (reject invalid
ephemeral pubkeys early)
- Add curve validation in crypto/ecies.GenerateShared to reject invalid
public keys before ECDH.
- Update RLPx PoC test to assert invalid curve points fail with
ErrInvalidPublicKey.

Motivation / Context
RLPx handshake uses ECIES decryption on unauthenticated network input.
Prior to this change, an invalid-curve ephemeral public key would
proceed into ECDH and only fail at MAC verification, returning
ErrInvalidMessage. This allows an oracle on decrypt success/failure and
leaves the code path vulnerable to invalid-curve/small-subgroup attacks.
The fix enforces IsOnCurve validation up front.
Updates the libsecp256k1 dependency to commit:
c0d9480fbbf8eccbd4be23ed27f6f2af6f3b211e

PR:
```
BenchmarkSign-24    	   57756	     21214 ns/op	     164 B/op	       3 allocs/op
BenchmarkRecover-24    	   37156	     33044 ns/op	      80 B/op	       1 allocs/op
BenchmarkEcrecoverSignature-24    	   36889	     32935 ns/op	      80 B/op	       1 allocs/op
BenchmarkVerifySignature-24    	   41163	     29207 ns/op	       0 B/op	       0 allocs/op
BenchmarkDecompressPubkey-24    	  318624	      4062 ns/op	     304 B/op	       6 allocs/op
```

Master:
```
BenchmarkSign-24    	   34509	     35330 ns/op	     164 B/op	       3 allocs/op
BenchmarkRecover-24    	   25418	     47725 ns/op	      80 B/op	       1 allocs/op
BenchmarkEcrecoverSignature-24    	   25735	     47591 ns/op	      80 B/op	       1 allocs/op
BenchmarkVerifySignature-24    	   29108	     41097 ns/op	       0 B/op	       0 allocs/op
BenchmarkDecompressPubkey-24    	  294747	      4143 ns/op	     304 B/op	       6 allocs/op
```

Performance seems to be improved significantly:
```
Sign-24      34.86µ ± 3%   21.66µ ± 2%  -37.86% (p=0.000 n=10)
Recover-24   46.14µ ± 3%   33.24µ ± 2%  -27.95% (p=0.000 n=10)
```
@eomti-wm eomti-wm self-assigned this Mar 13, 2026
@eomti-wm eomti-wm added bug Something isn't working enhancement New feature or request labels Mar 13, 2026
Copy link
Copy Markdown
Collaborator

@hominlee-wemade hominlee-wemade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown

@colinkim colinkim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown

@0xmhha 0xmhha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.