8000 net/http: reject newlines in chunk-size lines (backport to 1.19) by drehak · Pull Request #291 · golang-fips/go · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

net/http: reject newlines in chunk-size lines (backport to 1.19) #291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: go1.19-fips-release
Choose a base branch
from
+157 −0
Open

net/http: reject newlines in chunk-size lines (backport to 1.19) #291

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 157 additions & 0 deletions patches/013-reject-newlines-in-chunked-lines.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
From d31c805535f3fde95646ee4d87636aaaea66847b Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Wed, 26 Feb 2025 13:40:00 -0800
Subject: [PATCH] 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 #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>
---

diff --git a/src/net/http/internal/chunked.go b/src/net/http/internal/chunked.go
index 196b5d8..0b08a97 100644
--- a/src/net/http/internal/chunked.go
+++ b/src/net/http/internal/chunked.go
@@ -164,6 +164,19 @@
}
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 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 af79711..312f173 100644
--- a/src/net/http/internal/chunked_test.go
+++ b/src/net/http/internal/chunked_test.go
@@ -280,6 +280,33 @@
}
}

+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 89fcbd1..915055f 100644
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -6828,3 +6828,52 @@
}
}
}
+
+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, false, 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")
+ }
+ })
+ }
+}
Loading
0