From 673878bd403834f18619dcc8979e0e415b8a2b05 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Sat, 25 Jul 2026 17:04:26 +0300 Subject: [PATCH] security: VerifyAuthFrame rejects wrong-length peer public keys (panic-DoS) ed25519.Verify panics on a public key that is not exactly PublicKeySize bytes; VerifyAuthFrame received peerEdPubKey from a registry lookup or a caller-supplied HandshakeConfig and passed it straight to Verify. Adds a length guard plus fuzz coverage over the auth-frame and wire request decoders. Co-Authored-By: Claude Opus 5 --- registry/wire/zz_fuzz_reqdecode_test.go | 105 ++++++++++++++++++++++++ secure/secure.go | 6 ++ secure/zz_fuzz_authframe_test.go | 88 ++++++++++++++++++++ 3 files changed, 199 insertions(+) create mode 100644 registry/wire/zz_fuzz_reqdecode_test.go create mode 100644 secure/zz_fuzz_authframe_test.go diff --git a/registry/wire/zz_fuzz_reqdecode_test.go b/registry/wire/zz_fuzz_reqdecode_test.go new file mode 100644 index 0000000..e3bf4b9 --- /dev/null +++ b/registry/wire/zz_fuzz_reqdecode_test.go @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package wire_test + +import ( + "encoding/binary" + "testing" + + "github.com/pilot-protocol/common/registry/wire" +) + +// The sibling zz_fuzz_wire_test.go covers the framing layer and the +// client-side *response* decoders. These targets cover the three +// server-side *request* decoders, which are the ones fed directly from an +// unauthenticated TCP peer by accept.handleBinaryConn — the registry +// serves the whole fleet, so a panic here is a fleet-wide outage. + +func FuzzDecodeHeartbeatReqBounds(f *testing.F) { + f.Add([]byte{}) + f.Add(make([]byte, 67)) + f.Add(make([]byte, 68)) + f.Add(make([]byte, 69)) + f.Add(make([]byte, 4096)) + { + b := make([]byte, 68) + binary.BigEndian.PutUint32(b[:4], 0xFFFFFFFF) + f.Add(b) + } + + f.Fuzz(func(t *testing.T, payload []byte) { + req, err := wire.DecodeHeartbeatReq(payload) + if err != nil { + return + } + if len(payload) < 68 { + t.Fatalf("accepted a %d-byte heartbeat request", len(payload)) + } + // The decoded signature is handed straight to the verifier; it must + // always be exactly SignatureSize. + if len(req.Signature) != 64 { + t.Fatalf("decoded signature is %d bytes, want 64", len(req.Signature)) + } + }) +} + +func FuzzDecodeLookupReqBounds(f *testing.F) { + f.Add([]byte{}) + f.Add(make([]byte, 3)) + f.Add(make([]byte, 4)) + f.Add(make([]byte, 5)) + f.Add(make([]byte, 65535)) + + f.Fuzz(func(t *testing.T, payload []byte) { + if _, err := wire.DecodeLookupReq(payload); err != nil { + return + } + if len(payload) < 4 { + t.Fatalf("accepted a %d-byte lookup request", len(payload)) + } + }) +} + +func FuzzDecodeResolveReqBounds(f *testing.F) { + f.Add([]byte{}) + f.Add(make([]byte, 71)) + f.Add(make([]byte, 72)) + f.Add(make([]byte, 73)) + f.Add(make([]byte, 8192)) + { + b := make([]byte, 72) + binary.BigEndian.PutUint32(b[0:4], 1) + binary.BigEndian.PutUint32(b[4:8], 2) + f.Add(b) + } + + f.Fuzz(func(t *testing.T, payload []byte) { + _, _, sig, err := wire.DecodeResolveReq(payload) + if err != nil { + return + } + if len(payload) < 72 { + t.Fatalf("accepted a %d-byte resolve request", len(payload)) + } + // sig is passed to the signature verifier without further length + // handling, so the decoder owns this invariant. + if len(sig) != 64 { + t.Fatalf("decoded signature is %d bytes, want 64", len(sig)) + } + }) +} + +// FuzzDecodeErrorBounds covers the error-payload decoder, which trusts a +// wire-supplied uint16 length to slice its own buffer. +func FuzzDecodeErrorBounds(f *testing.F) { + f.Add([]byte{}) + f.Add([]byte{0x00}) + f.Add([]byte{0xFF, 0xFF}) + f.Add([]byte{0xFF, 0xFF, 'a', 'b'}) + f.Add([]byte{0x00, 0x04, 'a'}) + f.Add(wire.EncodeError("boom")) + + f.Fuzz(func(t *testing.T, payload []byte) { + _ = wire.DecodeError(payload) + }) +} diff --git a/secure/secure.go b/secure/secure.go index f3d94dc..37f5c27 100644 --- a/secure/secure.go +++ b/secure/secure.go @@ -630,6 +630,12 @@ func VerifyAuthFrame(frame []byte, peerEdPubKey ed25519.PublicKey, peerX25519Pub if len(frame) != AuthFrameLen { return 0, fmt.Errorf("auth frame wrong size: %d", len(frame)) } + // ed25519.Verify panics on a key that is not exactly PublicKeySize + // bytes. peerEdPubKey reaches here from a registry lookup or a + // caller-supplied HandshakeConfig, so validate before use. + if len(peerEdPubKey) != ed25519.PublicKeySize { + return 0, fmt.Errorf("peer public key wrong size: %d", len(peerEdPubKey)) + } peerNodeID := binary.BigEndian.Uint32(frame[0:4]) peerTS := binary.BigEndian.Uint64(frame[4:12]) diff --git a/secure/zz_fuzz_authframe_test.go b/secure/zz_fuzz_authframe_test.go new file mode 100644 index 0000000..a469d45 --- /dev/null +++ b/secure/zz_fuzz_authframe_test.go @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package secure_test + +import ( + "crypto/ed25519" + "encoding/binary" + "testing" + "time" + + "github.com/pilot-protocol/common/secure" +) + +// VerifyAuthFrame consumes the peer's auth frame after the ECDH exchange. +// The frame bytes are remote-controlled, and peerEdPubKey comes from a +// registry lookup or a caller-supplied HandshakeConfig — neither is +// guaranteed to be a well-formed Ed25519 key, and ed25519.Verify panics on +// a key that is not exactly PublicKeySize bytes. +func FuzzVerifyAuthFrame(f *testing.F) { + f.Add([]byte{}, 0) + f.Add(make([]byte, secure.AuthFrameLen), 0) + f.Add(make([]byte, secure.AuthFrameLen-1), 0) + f.Add(make([]byte, secure.AuthFrameLen+1), 0) + // Same frame against every malformed key length in the table below. + for i := 0; i < 6; i++ { + f.Add(make([]byte, secure.AuthFrameLen), i) + } + // A frame carrying a plausible (in-window) timestamp so the fuzzer gets + // past the skew gate and reaches the signature verify. + { + b := make([]byte, secure.AuthFrameLen) + binary.BigEndian.PutUint32(b[0:4], 1234) + binary.BigEndian.PutUint64(b[4:12], uint64(time.Now().Unix())) + f.Add(b, 0) + f.Add(b, 1) + } + + realPub, _, err := ed25519.GenerateKey(nil) + if err != nil { + f.Fatalf("keygen: %v", err) + } + // Index 0 is the only well-formed key; the rest are the shapes that + // reach ed25519.Verify from a corrupt or partially-populated record. + keys := []ed25519.PublicKey{ + realPub, + nil, + {}, + make([]byte, 1), + make([]byte, ed25519.PublicKeySize-1), + make([]byte, ed25519.PublicKeySize+1), + } + + peerX25519 := make([]byte, 32) + var nonceSeq uint64 + + f.Fuzz(func(t *testing.T, frame []byte, keyIdx int) { + if len(frame) > 4096 { + frame = frame[:4096] + } + if keyIdx < 0 { + keyIdx = -keyIdx + } + key := keys[keyIdx%len(keys)] + + // Run the frame as-is first: that covers the length guard and the + // timestamp-skew rejection. + if _, err := secure.VerifyAuthFrame(frame, key, peerX25519, time.Now()); err == nil { + t.Fatalf("VerifyAuthFrame accepted an unsigned frame (len=%d, keyLen=%d)", len(frame), len(key)) + } + + // Then run a normalised copy. Without this, almost every input dies + // at the timestamp gate or (for a repeated nonce) at the replay + // gate, and the Ed25519 verify — the part that is sensitive to key + // length — is never reached. A real peer always presents a fresh + // timestamp and nonce, so this is the shape that matters. + if len(frame) == secure.AuthFrameLen { + fresh := make([]byte, secure.AuthFrameLen) + copy(fresh, frame) + binary.BigEndian.PutUint64(fresh[4:12], uint64(time.Now().Unix())) + nonceSeq++ + binary.BigEndian.PutUint64(fresh[12:20], nonceSeq) + binary.BigEndian.PutUint64(fresh[20:28], uint64(keyIdx)) + if _, err := secure.VerifyAuthFrame(fresh, key, peerX25519, time.Now()); err == nil { + t.Fatalf("VerifyAuthFrame accepted an unsigned fresh frame (keyLen=%d)", len(key)) + } + } + }) +}