Skip to content

Commit a589fcd

Browse files
fengjianeomti-wm
authored andcommitted
fix: ECIES invalid-curve handling (ethereum#33669)
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.
1 parent 3e99e65 commit a589fcd

2 files changed

Lines changed: 60 additions & 0 deletions

File tree

crypto/ecies/ecies.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,9 @@ func (prv *PrivateKey) GenerateShared(pub *PublicKey, skLen, macLen int) (sk []b
122122
if prv.PublicKey.Curve != pub.Curve {
123123
return nil, ErrInvalidCurve
124124
}
125+
if pub.X == nil || pub.Y == nil || !pub.Curve.IsOnCurve(pub.X, pub.Y) {
126+
return nil, ErrInvalidPublicKey
127+
}
125128
if skLen+macLen > MaxSharedKeyLength(pub) {
126129
return nil, ErrSharedKeyTooBig
127130
}

p2p/rlpx/rlpx_oracle_poc_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package rlpx
2+
3+
import (
4+
"bytes"
5+
"errors"
6+
"testing"
7+
8+
"github.com/ethereum/go-ethereum/crypto"
9+
"github.com/ethereum/go-ethereum/crypto/ecies"
10+
)
11+
12+
func TestHandshakeECIESInvalidCurveOracle(t *testing.T) {
13+
initKey, err := crypto.GenerateKey()
14+
if err != nil {
15+
t.Fatal(err)
16+
}
17+
respKey, err := crypto.GenerateKey()
18+
if err != nil {
19+
t.Fatal(err)
20+
}
21+
22+
init := handshakeState{
23+
initiator: true,
24+
remote: ecies.ImportECDSAPublic(&respKey.PublicKey),
25+
}
26+
authMsg, err := init.makeAuthMsg(initKey)
27+
if err != nil {
28+
t.Fatal(err)
29+
}
30+
packet, err := init.sealEIP8(authMsg)
31+
if err != nil {
32+
t.Fatal(err)
33+
}
34+
35+
var recv handshakeState
36+
if _, err := recv.readMsg(new(authMsgV4), respKey, bytes.NewReader(packet)); err != nil {
37+
t.Fatalf("expected valid packet to decrypt: %v", err)
38+
}
39+
40+
tampered := append([]byte(nil), packet...)
41+
if len(tampered) < 2+65 {
42+
t.Fatalf("unexpected packet length %d", len(tampered))
43+
}
44+
tampered[2] = 0x04
45+
for i := 1; i < 65; i++ {
46+
tampered[2+i] = 0x00
47+
}
48+
49+
var recv2 handshakeState
50+
_, err = recv2.readMsg(new(authMsgV4), respKey, bytes.NewReader(tampered))
51+
if err == nil {
52+
t.Fatal("expected decryption failure for invalid curve point")
53+
}
54+
if !errors.Is(err, ecies.ErrInvalidPublicKey) {
55+
t.Fatalf("unexpected error: %v", err)
56+
}
57+
}

0 commit comments

Comments
 (0)