Add vanilla parity harness and harden server boundaries

internal/protocol/buffer.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"io"
 	"math"
+	"unicode/utf8"
 )
 
 // ErrShortBuffer is returned when a read would exceed the buffer's contents.
@@ -119,6 +120,12 @@ func (r *Reader) String() (string, error) {
 	if err != nil {
 		return "", err
 	}
+	if !utf8.Valid(b) {
+		return "", ErrInvalidString
+	}
+	if utf8.RuneCount(b) > MaxStringLen {
+		return "", ErrStringTooLong
+	}
 	return string(b), nil
 }
 

internal/protocol/frame.go

@@ -46,11 +46,11 @@ func ReadPacket(br *bufio.Reader, threshold int32) (Packet, error) {
 	if threshold < 0 {
 		return parseIDBody(frame)
 	}
-	return parseCompressed(frame)
+	return parseCompressed(frame, threshold)
 }
 
 // parseCompressed handles a frame that begins with a Data Length VarInt.
-func parseCompressed(frame []byte) (Packet, error) {
+func parseCompressed(frame []byte, threshold int32) (Packet, error) {
 	r := NewReader(frame)
 	dataLen, err := r.VarInt()
 	if err != nil {
@@ -60,22 +60,40 @@ func parseCompressed(frame []byte) (Packet, error) {
 
 	if dataLen == 0 {
 		// Stored uncompressed.
+		if len(payload) >= int(threshold) {
+			return Packet{}, ErrBadCompression
+		}
 		return parseIDBody(payload)
 	}
 	if dataLen < 0 || int(dataLen) > MaxPacketSize {
 		return Packet{}, ErrPacketTooLarge
 	}
+	if dataLen < threshold {
+		return Packet{}, ErrBadCompression
+	}
 
-	zr, err := zlib.NewReader(bytes.NewReader(payload))
+	compressed := bytes.NewReader(payload)
+	zr, err := zlib.NewReader(compressed)
 	if err != nil {
 		return Packet{}, err
 	}
-	defer zr.Close()
+	if multistream, ok := zr.(interface{ Multistream(bool) }); ok {
+		multistream.Multistream(false)
+	}
 
 	out := make([]byte, dataLen)
 	if _, err := io.ReadFull(zr, out); err != nil {
+		zr.Close()
 		return Packet{}, err
 	}
+	var extra [1]byte
+	if n, err := zr.Read(extra[:]); n != 0 || err != io.EOF {
+		zr.Close()
+		return Packet{}, ErrBadCompression
+	}
+	if err := zr.Close(); err != nil || compressed.Len() != 0 {
+		return Packet{}, ErrBadCompression
+	}
 	return parseIDBody(out)
 }
 
@@ -92,8 +110,18 @@ func parseIDBody(buf []byte) (Packet, error) {
 // WritePacket writes one frame to w with the given ID and body, using the
 // uncompressed format when threshold < 0 and the compressed format otherwise.
 func WritePacket(w io.Writer, threshold int32, id int32, body []byte) error {
-	_, err := w.Write(AppendPacket(nil, threshold, id, body))
-	return err
+	frame := AppendPacket(nil, threshold, id, body)
+	for len(frame) > 0 {
+		n, err := w.Write(frame)
+		if err != nil {
+			return err
+		}
+		if n <= 0 || n > len(frame) {
+			return io.ErrShortWrite
+		}
+		frame = frame[n:]
+	}
+	return nil
 }
 
 // AppendPacket appends one fully-framed packet to dst and returns the result.

internal/protocol/frame_test.go

@@ -0,0 +1,99 @@
+package protocol
+
+import (
+	"bufio"
+	"bytes"
+	"errors"
+	"io"
+	"testing"
+)
+
+func TestReadPacketCompressionThreshold(t *testing.T) {
+	for _, tc := range []struct {
+		name      string
+		writeAt   int32
+		readAt    int32
+		wantError error
+	}{
+		{name: "compressed at threshold", writeAt: 4, readAt: 4},
+		{name: "compressed below threshold", writeAt: 4, readAt: 9, wantError: ErrBadCompression},
+		{name: "uncompressed below threshold", writeAt: 16, readAt: 16},
+		{name: "uncompressed at threshold", writeAt: 16, readAt: 4, wantError: ErrBadCompression},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			frame := AppendPacket(nil, tc.writeAt, 3, []byte("payload"))
+			pkt, err := ReadPacket(bufio.NewReader(bytes.NewReader(frame)), tc.readAt)
+			if !errors.Is(err, tc.wantError) {
+				t.Fatalf("ReadPacket error = %v, want %v", err, tc.wantError)
+			}
+			if tc.wantError == nil && (pkt.ID != 3 || string(pkt.Data) != "payload") {
+				t.Fatalf("packet = id %d data %q", pkt.ID, pkt.Data)
+			}
+		})
+	}
+}
+
+func TestReadPacketRejectsWrongDecompressedLength(t *testing.T) {
+	frame := AppendPacket(nil, 1, 3, []byte("payload"))
+	r := NewReader(frame)
+	length, err := r.VarInt()
+	if err != nil {
+		t.Fatal(err)
+	}
+	payload := append([]byte(nil), frame[len(frame)-int(length):]...)
+	payload[0]++
+	bad := AppendVarInt(nil, int32(len(payload)))
+	bad = append(bad, payload...)
+	if _, err := ReadPacket(bufio.NewReader(bytes.NewReader(bad)), 1); err == nil {
+		t.Fatal("accepted compressed payload shorter than its declared length")
+	}
+}
+
+func TestReadPacketRejectsTrailingCompressedData(t *testing.T) {
+	frame := AppendPacket(nil, 1, 3, []byte("payload"))
+	r := NewReader(frame)
+	length, err := r.VarInt()
+	if err != nil {
+		t.Fatal(err)
+	}
+	payload := append([]byte(nil), frame[len(frame)-int(length):]...)
+	payload = append(payload, 0)
+	bad := AppendVarInt(nil, int32(len(payload)))
+	bad = append(bad, payload...)
+	if _, err := ReadPacket(bufio.NewReader(bytes.NewReader(bad)), 1); !errors.Is(err, ErrBadCompression) {
+		t.Fatalf("error = %v, want ErrBadCompression", err)
+	}
+}
+
+type shortWriter struct{ buf bytes.Buffer }
+
+func (w *shortWriter) Write(p []byte) (int, error) {
+	if len(p) > 2 {
+		p = p[:2]
+	}
+	return w.buf.Write(p)
+}
+
+func TestWritePacketCompletesShortWrites(t *testing.T) {
+	w := new(shortWriter)
+	if err := WritePacket(w, -1, 7, []byte("body")); err != nil {
+		t.Fatal(err)
+	}
+	pkt, err := ReadPacket(bufio.NewReader(bytes.NewReader(w.buf.Bytes())), -1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if pkt.ID != 7 || string(pkt.Data) != "body" {
+		t.Fatalf("packet = id %d data %q", pkt.ID, pkt.Data)
+	}
+}
+
+type zeroWriter struct{}
+
+func (zeroWriter) Write([]byte) (int, error) { return 0, nil }
+
+func TestWritePacketRejectsNoProgress(t *testing.T) {
+	if err := WritePacket(zeroWriter{}, -1, 1, nil); !errors.Is(err, io.ErrShortWrite) {
+		t.Fatalf("error = %v, want io.ErrShortWrite", err)
+	}
+}

internal/protocol/fuzz_test.go

@@ -0,0 +1,16 @@
+package protocol
+
+import (
+	"bufio"
+	"bytes"
+	"testing"
+)
+
+func FuzzReadPacketNeverPanics(f *testing.F) {
+	f.Add(AppendPacket(nil, -1, 0, nil))
+	f.Add(AppendPacket(nil, 1, 42, []byte("payload")))
+	f.Fuzz(func(t *testing.T, input []byte) {
+		_, _ = ReadPacket(bufio.NewReader(bytes.NewReader(input)), 256)
+		_, _ = ReadPacket(bufio.NewReader(bytes.NewReader(input)), -1)
+	})
+}

internal/protocol/types.go

@@ -64,8 +64,13 @@ var (
 	ErrVarIntTooBig = errors.New("protocol: varint is too big")
 	// ErrStringTooLong is returned when a string exceeds MaxStringLen.
 	ErrStringTooLong = errors.New("protocol: string too long")
+	// ErrInvalidString is returned for protocol strings that are not UTF-8.
+	ErrInvalidString = errors.New("protocol: invalid UTF-8 string")
 	// ErrPacketTooLarge is returned when a packet length exceeds MaxPacketSize.
 	ErrPacketTooLarge = errors.New("protocol: packet too large")
+	// ErrBadCompression is returned when a frame violates the negotiated
+	// compression threshold or its stream does not match the declared length.
+	ErrBadCompression = errors.New("protocol: invalid compressed packet")
 )
 
 // ReadVarInt reads a 32-bit VarInt from r, returning the value and the number