[release-branch.go1.24] net/http: reject newlines in chunk-size lines

Unlike request headers, where we are allowed to leniently accept
a bare LF in place of a CRLF, chunked bodies must always use CRLF
line terminators. We were already enforcing this for chunk-data lines;
do so for chunk-size lines as well. Also reject bare CRs anywhere
other than as part of the CRLF terminator.

Fixes CVE-2025-22871
Fixes #72011
For #71988

Change-Id: Ib0e21af5a8ba28c2a1ca52b72af8e2265ec79e4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/652998
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit d31c805535f3fde95646ee4d87636aaaea66847b)
Reviewed-on: https://go-review.googlesource.com/c/go/+/657056

diff --git a/src/net/http/internal/chunked.go b/src/net/http/internal/chunked.go
index 196b5d892589ab104b9acf80b12e7237fb684bf2..0b08a97a0831ecc768effee97fb341c827ccf8e6 100644 (file)
--- a/src/net/http/internal/chunked.go
+++ b/src/net/http/internal/chunked.go
@@ -164,6 +164,19 @@ func readChunkLine(b *bufio.Reader) ([]byte, error) {
                }
                return nil, err
        }
+
+       // RFC 9112 permits parsers to accept a bare \n as a line ending in headers,
+       // but not in chunked encoding lines. See https://www.rfc-editor.org/errata/eid7633,
+       // which explicitly rejects a clarification permitting \n as a chunk terminator.
+       //
+       // Verify that the line ends in a CRLF, and that no CRs appear before the end.
+       if idx := bytes.IndexByte(p, '\r'); idx == -1 {
+               return nil, errors.New("chunked line ends with bare LF")
+       } else if idx != len(p)-2 {
+               return nil, errors.New("invalid CR in chunked line")
+       }
+       p = p[:len(p)-2] // trim CRLF
+
        if len(p) >= maxLineLength {
                return nil, ErrLineTooLong
        }
@@ -171,14 +184,14 @@ func readChunkLine(b *bufio.Reader) ([]byte, error) {
 }
 
 func trimTrailingWhitespace(b []byte) []byte {
-       for len(b) > 0 && isASCIISpace(b[len(b)-1]) {
+       for len(b) > 0 && isOWS(b[len(b)-1]) {
                b = b[:len(b)-1]
        }
        return b
 }
 
-func isASCIISpace(b byte) bool {
-       return b == ' ' || b == '\t' || b == '\n' || b == '\r'
+func isOWS(b byte) bool {
+       return b == ' ' || b == '\t'
 }
 
 var semi = []byte(";")

diff --git a/src/net/http/internal/chunked_test.go b/src/net/http/internal/chunked_test.go
index af79711781a7edaf2479e2cc11d2e2233ee0cca9..312f1734a6385caaf09c987beb4f12a3a05f1928 100644 (file)
--- a/src/net/http/internal/chunked_test.go
+++ b/src/net/http/internal/chunked_test.go
@@ -280,6 +280,33 @@ func TestChunkReaderByteAtATime(t *testing.T) {
        }
 }
 
+func TestChunkInvalidInputs(t *testing.T) {
+       for _, test := range []struct {
+               name string
+               b    string
+       }{{
+               name: "bare LF in chunk size",
+               b:    "1\na\r\n0\r\n",
+       }, {
+               name: "extra LF in chunk size",
+               b:    "1\r\r\na\r\n0\r\n",
+       }, {
+               name: "bare LF in chunk data",
+               b:    "1\r\na\n0\r\n",
+       }, {
+               name: "bare LF in chunk extension",
+               b:    "1;\na\r\n0\r\n",
+       }} {
+               t.Run(test.name, func(t *testing.T) {
+                       r := NewChunkedReader(strings.NewReader(test.b))
+                       got, err := io.ReadAll(r)
+                       if err == nil {
+                               t.Fatalf("unexpectedly parsed invalid chunked data:\n%q", got)
+                       }
+               })
+       }
+}
+
 type funcReader struct {
        f   func(iteration int) ([]byte, error)
        i   int

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index 517cfdd92ebdc52978876a7e1431c04aeb7de6b0..dd0e566eaf03b165e655c1f0c50ba9b1c659c077 100644 (file)
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -7372,3 +7372,52 @@ func testServerTLSNextProtos(t *testing.T, mode testMode) {
                t.Fatalf("after running test: original NextProtos slice = %v, want %v", nextProtos, wantNextProtos)
        }
 }
+
+func TestInvalidChunkedBodies(t *testing.T) {
+       for _, test := range []struct {
+               name string
+               b    string
+       }{{
+               name: "bare LF in chunk size",
+               b:    "1\na\r\n0\r\n\r\n",
+       }, {
+               name: "bare LF at body end",
+               b:    "1\r\na\r\n0\r\n\n",
+       }} {
+               t.Run(test.name, func(t *testing.T) {
+                       reqc := make(chan error)
+                       ts := newClientServerTest(t, http1Mode, HandlerFunc(func(w ResponseWriter, r *Request) {
+                               got, err := io.ReadAll(r.Body)
+                               if err == nil {
+                                       t.Logf("read body: %q", got)
+                               }
+                               reqc <- err
+                       })).ts
+
+                       serverURL, err := url.Parse(ts.URL)
+                       if err != nil {
+                               t.Fatal(err)
+                       }
+
+                       conn, err := net.Dial("tcp", serverURL.Host)
+                       if err != nil {
+                               t.Fatal(err)
+                       }
+
+                       if _, err := conn.Write([]byte(
+                               "POST / HTTP/1.1\r\n" +
+                                       "Host: localhost\r\n" +
+                                       "Transfer-Encoding: chunked\r\n" +
+                                       "Connection: close\r\n" +
+                                       "\r\n" +
+                                       test.b)); err != nil {
+                               t.Fatal(err)
+                       }
+                       conn.(*net.TCPConn).CloseWrite()
+
+                       if err := <-reqc; err == nil {
+                               t.Errorf("server handler: io.ReadAll(r.Body) succeeded, want error")
+                       }
+               })
+       }
+}