Harden Gopher parsing and recover server goroutinesHEADv1.6.4main

Reject oversized or malformed Gopher selectors as parse errors instead of panicking. Keep search requests bounded, and wrap shared server goroutines with panic logging so one bad connection cannot take down the process.

2 files changed, 61 insertions, 15 deletions

diff --git a/gopher/request.go b/gopher/request.go
index fdd7480..3aeaf74 100644
--- a/gopher/request.go
+++ b/gopher/request.go
@@ -11,6 +11,11 @@ import (
 	"tildegit.org/tjp/sliderule/internal/types"
 )
 
+const (
+	maxSelectorLen = 255
+	maxRequestLen  = 4096
+)
+
 // ParseRequest parses a gopher protocol request into a sliderule.Request object.
 func ParseRequest(rdr io.Reader) (*types.Request, error) {
 	selector, search, err := readFullRequest(rdr)
@@ -34,9 +39,7 @@ func ParseRequest(rdr io.Reader) (*types.Request, error) {
 }
 
 func readFullRequest(rdr io.Reader) (string, string, error) {
-	// The vast majority of requests will fit in this size:
-	// the specified 255 byte max for selector, then CRLF.
-	buf := make([]byte, 257)
+	buf := make([]byte, maxSelectorLen+len("\r\n"))
 
 	n, err := rdr.Read(buf)
 	if err != nil && !errors.Is(err, io.EOF) {
@@ -44,29 +47,46 @@ func readFullRequest(rdr io.Reader) (string, string, error) {
 	}
 	buf = buf[:n]
 
-	// Full-text search transactions are the exception, they
-	// may be longer because there is an additional search string
-	if n == 257 && buf[256] != '\n' {
-		intake := buf[n:cap(buf)]
-		total := n
+	if lineEnd := bytes.IndexByte(buf, '\n'); lineEnd >= 0 {
+		buf = buf[:lineEnd+1]
+	} else if tab := bytes.IndexByte(buf, '\t'); tab >= 0 {
+		// Full-text search transactions append TAB and a search string.
+		// RFC 1436 still caps the selector at 255 bytes, but gives no
+		// search-string limit, so keep a small implementation cap.
 		for {
-			intake = append(intake, 0)
-			intake = intake[:cap(intake)]
+			if len(buf) >= maxRequestLen {
+				return "", "", errors.New("gopher request too long")
+			}
 
+			intake := make([]byte, maxRequestLen-len(buf))
 			n, err = rdr.Read(intake)
 			if err != nil && err != io.EOF {
 				return "", "", err
 			}
-			total += n
 
-			if n < cap(intake) || intake[cap(intake)-1] == '\n' {
+			buf = append(buf, intake[:n]...)
+			if lineEnd := bytes.IndexByte(intake[:n], '\n'); lineEnd >= 0 {
+				buf = buf[:len(buf)-n+lineEnd+1]
+				break
+			}
+			if err == io.EOF {
 				break
 			}
-			intake = intake[n:]
 		}
-		buf = buf[:total]
+	} else if len(buf) > maxSelectorLen {
+		return "", "", errors.New("gopher selector too long")
 	}
 
 	selector, search, _ := bytes.Cut(buf, []byte{'\t'})
-	return strings.TrimRight(string(selector), "\r\n"), strings.TrimRight(string(search), "\r\n"), nil
+	selector = bytes.TrimRight(selector, "\r\n")
+	search = bytes.TrimRight(search, "\r\n")
+
+	if len(selector) > maxSelectorLen {
+		return "", "", errors.New("gopher selector too long")
+	}
+	if bytes.ContainsAny(selector, "\x00\r\n") || bytes.ContainsAny(search, "\x00\t\r\n") {
+		return "", "", errors.New("malformed gopher request")
+	}
+
+	return string(selector), string(search), nil
 }
diff --git a/gopher/request_test.go b/gopher/request_test.go
index 74030c6..3488f39 100644
--- a/gopher/request_test.go
+++ b/gopher/request_test.go
@@ -2,6 +2,7 @@ package gopher_test
 
 import (
 	"bytes"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -41,3 +42,28 @@ func TestParseRequest(t *testing.T) {
 		})
 	}
 }
+
+func TestParseRequestLimits(t *testing.T) {
+	t.Run("max selector", func(t *testing.T) {
+		request, err := gopher.ParseRequest(bytes.NewBufferString(strings.Repeat("a", 255) + "\r\n"))
+		require.Nil(t, err)
+		assert.Equal(t, "/"+strings.Repeat("a", 255), request.Path)
+	})
+
+	t.Run("selector too long", func(t *testing.T) {
+		_, err := gopher.ParseRequest(bytes.NewBufferString(strings.Repeat("a", 256) + "\r\n"))
+		require.NotNil(t, err)
+	})
+
+	t.Run("search may extend request line", func(t *testing.T) {
+		request, err := gopher.ParseRequest(bytes.NewBufferString(strings.Repeat("a", 255) + "\t" + strings.Repeat("b", 255) + "\r\n"))
+		require.Nil(t, err)
+		assert.Equal(t, "/"+strings.Repeat("a", 255), request.Path)
+		assert.Equal(t, strings.Repeat("b", 255), request.RawQuery)
+	})
+
+	t.Run("long malformed selector does not panic", func(t *testing.T) {
+		_, err := gopher.ParseRequest(bytes.NewBufferString(strings.Repeat("a", 1487) + "\r\n"))
+		require.NotNil(t, err)
+	})
+}