From 5f52108d446d14ef44de8c683dec34e9327d0395 Mon Sep 17 00:00:00 2001 From: David Benoit Date: Tue, 18 Jun 2024 08:18:10 -0400 Subject: [PATCH 1/3] Backport fix for CVE-2023-45290 --- .../004-net-textproto-mime-multipart-a.patch | 266 ++++++++++++++++++ 1 file changed, 266 insertions(+) create mode 100644 patches/004-net-textproto-mime-multipart-a.patch diff --git a/patches/004-net-textproto-mime-multipart-a.patch b/patches/004-net-textproto-mime-multipart-a.patch new file mode 100644 index 0000000000..4cd7a91ff7 --- /dev/null +++ b/patches/004-net-textproto-mime-multipart-a.patch @@ -0,0 +1,266 @@ +From c911efa18163bea9ef152be227b07c5cac0e449d Mon Sep 17 00:00:00 2001 +From: Damien Neil +Date: Tue, 16 Jan 2024 15:37:52 -0800 +Subject: [PATCH] [release-branch.go1.21] net/textproto, mime/multipart: avoid + unbounded read in MIME header + +mime/multipart.Reader.ReadForm allows specifying the maximum amount +of memory that will be consumed by the form. While this limit is +correctly applied to the parsed form data structure, it was not +being applied to individual header lines in a form. + +For example, when presented with a form containing a header line +that never ends, ReadForm will continue to read the line until it +runs out of memory. + +Limit the amount of data consumed when reading a header. + +Fixes CVE-2023-45290 +Fixes #65389 +For #65383 + +Change-Id: I7f9264d25752009e95f6b2c80e3d76aaf321d658 +Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2134435 +Reviewed-by: Roland Shoemaker +Reviewed-by: Tatiana Bradley +Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2173776 +Reviewed-by: Carlos Amedee +Reviewed-on: https://go-review.googlesource.com/c/go/+/569240 +Auto-Submit: Michael Knyszek +LUCI-TryBot-Result: Go LUCI +Reviewed-by: Carlos Amedee +--- + src/mime/multipart/formdata_test.go | 42 +++++++++++++++++++++++++ + src/net/textproto/reader.go | 48 ++++++++++++++++++++--------- + src/net/textproto/reader_test.go | 12 ++++++++ + 3 files changed, 87 insertions(+), 15 deletions(-) + +diff --git a/src/mime/multipart/formdata_test.go b/src/mime/multipart/formdata_test.go +index a618a64beb..dd44ae4e21 100644 +--- a/src/mime/multipart/formdata_test.go ++++ b/src/mime/multipart/formdata_test.go +@@ -421,6 +421,48 @@ func TestReadFormLimits(t *testing.T) { + } + } + ++func TestReadFormEndlessHeaderLine(t *testing.T) { ++ for _, test := range []struct { ++ name string ++ prefix string ++ }{{ ++ name: "name", ++ prefix: "X-", ++ }, { ++ name: "value", ++ prefix: "X-Header: ", ++ }, { ++ name: "continuation", ++ prefix: "X-Header: foo\r\n ", ++ }} { ++ t.Run(test.name, func(t *testing.T) { ++ const eol = "\r\n" ++ s := `--boundary` + eol ++ s += `Content-Disposition: form-data; name="a"` + eol ++ s += `Content-Type: text/plain` + eol ++ s += test.prefix ++ fr := io.MultiReader( ++ strings.NewReader(s), ++ neverendingReader('X'), ++ ) ++ r := NewReader(fr, "boundary") ++ _, err := r.ReadForm(1 << 20) ++ if err != ErrMessageTooLarge { ++ t.Fatalf("ReadForm(1 << 20): %v, want ErrMessageTooLarge", err) ++ } ++ }) ++ } ++} ++ ++type neverendingReader byte ++ ++func (r neverendingReader) Read(p []byte) (n int, err error) { ++ for i := range p { ++ p[i] = byte(r) ++ } ++ return len(p), nil ++} ++ + func BenchmarkReadForm(b *testing.B) { + for _, test := range []struct { + name string +diff --git a/src/net/textproto/reader.go b/src/net/textproto/reader.go +index fc2590b1cd..fcd1a011ac 100644 +--- a/src/net/textproto/reader.go ++++ b/src/net/textproto/reader.go +@@ -16,6 +16,10 @@ import ( + "sync" + ) + ++// TODO: This should be a distinguishable error (ErrMessageTooLarge) ++// to allow mime/multipart to detect it. ++var errMessageTooLarge = errors.New("message too large") ++ + // A Reader implements convenience methods for reading requests + // or responses from a text protocol network connection. + type Reader struct { +@@ -36,20 +40,23 @@ func NewReader(r *bufio.Reader) *Reader { + // ReadLine reads a single line from r, + // eliding the final \n or \r\n from the returned string. + func (r *Reader) ReadLine() (string, error) { +- line, err := r.readLineSlice() ++ line, err := r.readLineSlice(-1) + return string(line), err + } + + // ReadLineBytes is like ReadLine but returns a []byte instead of a string. + func (r *Reader) ReadLineBytes() ([]byte, error) { +- line, err := r.readLineSlice() ++ line, err := r.readLineSlice(-1) + if line != nil { + line = bytes.Clone(line) + } + return line, err + } + +-func (r *Reader) readLineSlice() ([]byte, error) { ++// readLineSlice reads a single line from r, ++// up to lim bytes long (or unlimited if lim is less than 0), ++// eliding the final \r or \r\n from the returned string. ++func (r *Reader) readLineSlice(lim int64) ([]byte, error) { + r.closeDot() + var line []byte + for { +@@ -57,6 +64,9 @@ func (r *Reader) readLineSlice() ([]byte, error) { + if err != nil { + return nil, err + } ++ if lim >= 0 && int64(len(line))+int64(len(l)) > lim { ++ return nil, errMessageTooLarge ++ } + // Avoid the copy if the first call produced a full line. + if line == nil && !more { + return l, nil +@@ -88,7 +98,7 @@ func (r *Reader) readLineSlice() ([]byte, error) { + // + // Empty lines are never continued. + func (r *Reader) ReadContinuedLine() (string, error) { +- line, err := r.readContinuedLineSlice(noValidation) ++ line, err := r.readContinuedLineSlice(-1, noValidation) + return string(line), err + } + +@@ -109,7 +119,7 @@ func trim(s []byte) []byte { + // ReadContinuedLineBytes is like ReadContinuedLine but + // returns a []byte instead of a string. + func (r *Reader) ReadContinuedLineBytes() ([]byte, error) { +- line, err := r.readContinuedLineSlice(noValidation) ++ line, err := r.readContinuedLineSlice(-1, noValidation) + if line != nil { + line = bytes.Clone(line) + } +@@ -120,13 +130,14 @@ func (r *Reader) ReadContinuedLineBytes() ([]byte, error) { + // returning a byte slice with all lines. The validateFirstLine function + // is run on the first read line, and if it returns an error then this + // error is returned from readContinuedLineSlice. +-func (r *Reader) readContinuedLineSlice(validateFirstLine func([]byte) error) ([]byte, error) { ++// It reads up to lim bytes of data (or unlimited if lim is less than 0). ++func (r *Reader) readContinuedLineSlice(lim int64, validateFirstLine func([]byte) error) ([]byte, error) { + if validateFirstLine == nil { + return nil, fmt.Errorf("missing validateFirstLine func") + } + + // Read the first line. +- line, err := r.readLineSlice() ++ line, err := r.readLineSlice(lim) + if err != nil { + return nil, err + } +@@ -154,13 +165,21 @@ func (r *Reader) readContinuedLineSlice(validateFirstLine func([]byte) error) ([ + // copy the slice into buf. + r.buf = append(r.buf[:0], trim(line)...) + ++ if lim < 0 { ++ lim = math.MaxInt64 ++ } ++ lim -= int64(len(r.buf)) ++ + // Read continuation lines. + for r.skipSpace() > 0 { +- line, err := r.readLineSlice() ++ r.buf = append(r.buf, ' ') ++ if int64(len(r.buf)) >= lim { ++ return nil, errMessageTooLarge ++ } ++ line, err := r.readLineSlice(lim - int64(len(r.buf))) + if err != nil { + break + } +- r.buf = append(r.buf, ' ') + r.buf = append(r.buf, trim(line)...) + } + return r.buf, nil +@@ -507,7 +526,8 @@ func readMIMEHeader(r *Reader, maxMemory, maxHeaders int64) (MIMEHeader, error) + + // The first line cannot start with a leading space. + if buf, err := r.R.Peek(1); err == nil && (buf[0] == ' ' || buf[0] == '\t') { +- line, err := r.readLineSlice() ++ const errorLimit = 80 // arbitrary limit on how much of the line we'll quote ++ line, err := r.readLineSlice(errorLimit) + if err != nil { + return m, err + } +@@ -515,7 +535,7 @@ func readMIMEHeader(r *Reader, maxMemory, maxHeaders int64) (MIMEHeader, error) + } + + for { +- kv, err := r.readContinuedLineSlice(mustHaveFieldNameColon) ++ kv, err := r.readContinuedLineSlice(maxMemory, mustHaveFieldNameColon) + if len(kv) == 0 { + return m, err + } +@@ -544,7 +564,7 @@ func readMIMEHeader(r *Reader, maxMemory, maxHeaders int64) (MIMEHeader, error) + + maxHeaders-- + if maxHeaders < 0 { +- return nil, errors.New("message too large") ++ return nil, errMessageTooLarge + } + + // Skip initial spaces in value. +@@ -557,9 +577,7 @@ func readMIMEHeader(r *Reader, maxMemory, maxHeaders int64) (MIMEHeader, error) + } + maxMemory -= int64(len(value)) + if maxMemory < 0 { +- // TODO: This should be a distinguishable error (ErrMessageTooLarge) +- // to allow mime/multipart to detect it. +- return m, errors.New("message too large") ++ return m, errMessageTooLarge + } + if vv == nil && len(strs) > 0 { + // More than likely this will be a single-element key. +diff --git a/src/net/textproto/reader_test.go b/src/net/textproto/reader_test.go +index 696ae406f3..26ff617470 100644 +--- a/src/net/textproto/reader_test.go ++++ b/src/net/textproto/reader_test.go +@@ -36,6 +36,18 @@ func TestReadLine(t *testing.T) { + } + } + ++func TestReadLineLongLine(t *testing.T) { ++ line := strings.Repeat("12345", 10000) ++ r := reader(line + "\r\n") ++ s, err := r.ReadLine() ++ if err != nil { ++ t.Fatalf("Line 1: %v", err) ++ } ++ if s != line { ++ t.Fatalf("%v-byte line does not match expected %v-byte line", len(s), len(line)) ++ } ++} ++ + func TestReadContinuedLine(t *testing.T) { + r := reader("line1\nline\n 2\nline3\n") + s, err := r.ReadContinuedLine() +-- +2.45.1 + From b07986ae8bd0f81e19f8935bf1e297326a4f6cf5 Mon Sep 17 00:00:00 2001 From: David Benoit Date: Mon, 15 Jul 2024 08:40:52 -0400 Subject: [PATCH 2/3] rebase patch --- ... 009-net-textproto-mime-multipart-a.patch} | 41 ++++++++----------- 1 file changed, 18 insertions(+), 23 deletions(-) rename patches/{004-net-textproto-mime-multipart-a.patch => 009-net-textproto-mime-multipart-a.patch} (89%) diff --git a/patches/004-net-textproto-mime-multipart-a.patch b/patches/009-net-textproto-mime-multipart-a.patch similarity index 89% rename from patches/004-net-textproto-mime-multipart-a.patch rename to patches/009-net-textproto-mime-multipart-a.patch index 4cd7a91ff7..c3834f65e9 100644 --- a/patches/004-net-textproto-mime-multipart-a.patch +++ b/patches/009-net-textproto-mime-multipart-a.patch @@ -36,7 +36,7 @@ Reviewed-by: Carlos Amedee 3 files changed, 87 insertions(+), 15 deletions(-) diff --git a/src/mime/multipart/formdata_test.go b/src/mime/multipart/formdata_test.go -index a618a64beb..dd44ae4e21 100644 +index c78eeb7a12..f729da6954 100644 --- a/src/mime/multipart/formdata_test.go +++ b/src/mime/multipart/formdata_test.go @@ -421,6 +421,48 @@ func TestReadFormLimits(t *testing.T) { @@ -89,7 +89,7 @@ index a618a64beb..dd44ae4e21 100644 for _, test := range []struct { name string diff --git a/src/net/textproto/reader.go b/src/net/textproto/reader.go -index fc2590b1cd..fcd1a011ac 100644 +index 1ad4416ee1..fe3010d207 100644 --- a/src/net/textproto/reader.go +++ b/src/net/textproto/reader.go @@ -16,6 +16,10 @@ import ( @@ -103,7 +103,7 @@ index fc2590b1cd..fcd1a011ac 100644 // A Reader implements convenience methods for reading requests // or responses from a text protocol network connection. type Reader struct { -@@ -36,20 +40,23 @@ func NewReader(r *bufio.Reader) *Reader { +@@ -36,13 +40,13 @@ func NewReader(r *bufio.Reader) *Reader { // ReadLine reads a single line from r, // eliding the final \n or \r\n from the returned string. func (r *Reader) ReadLine() (string, error) { @@ -117,20 +117,18 @@ index fc2590b1cd..fcd1a011ac 100644 - line, err := r.readLineSlice() + line, err := r.readLineSlice(-1) if line != nil { - line = bytes.Clone(line) - } + buf := make([]byte, len(line)) + copy(buf, line) +@@ -51,7 +55,7 @@ func (r *Reader) ReadLineBytes() ([]byte, error) { return line, err } -func (r *Reader) readLineSlice() ([]byte, error) { -+// readLineSlice reads a single line from r, -+// up to lim bytes long (or unlimited if lim is less than 0), -+// eliding the final \r or \r\n from the returned string. +func (r *Reader) readLineSlice(lim int64) ([]byte, error) { r.closeDot() var line []byte for { -@@ -57,6 +64,9 @@ func (r *Reader) readLineSlice() ([]byte, error) { +@@ -59,6 +63,9 @@ func (r *Reader) readLineSlice() ([]byte, error) { if err != nil { return nil, err } @@ -140,7 +138,7 @@ index fc2590b1cd..fcd1a011ac 100644 // Avoid the copy if the first call produced a full line. if line == nil && !more { return l, nil -@@ -88,7 +98,7 @@ func (r *Reader) readLineSlice() ([]byte, error) { +@@ -90,7 +97,7 @@ func (r *Reader) readLineSlice() ([]byte, error) { // // Empty lines are never continued. func (r *Reader) ReadContinuedLine() (string, error) { @@ -149,16 +147,16 @@ index fc2590b1cd..fcd1a011ac 100644 return string(line), err } -@@ -109,7 +119,7 @@ func trim(s []byte) []byte { +@@ -111,7 +118,7 @@ func trim(s []byte) []byte { // ReadContinuedLineBytes is like ReadContinuedLine but // returns a []byte instead of a string. func (r *Reader) ReadContinuedLineBytes() ([]byte, error) { - line, err := r.readContinuedLineSlice(noValidation) + line, err := r.readContinuedLineSlice(-1, noValidation) if line != nil { - line = bytes.Clone(line) - } -@@ -120,13 +130,14 @@ func (r *Reader) ReadContinuedLineBytes() ([]byte, error) { + buf := make([]byte, len(line)) + copy(buf, line) +@@ -124,13 +131,14 @@ func (r *Reader) ReadContinuedLineBytes() ([]byte, error) { // returning a byte slice with all lines. The validateFirstLine function // is run on the first read line, and if it returns an error then this // error is returned from readContinuedLineSlice. @@ -175,7 +173,7 @@ index fc2590b1cd..fcd1a011ac 100644 if err != nil { return nil, err } -@@ -154,13 +165,21 @@ func (r *Reader) readContinuedLineSlice(validateFirstLine func([]byte) error) ([ +@@ -158,13 +166,21 @@ func (r *Reader) readContinuedLineSlice(validateFirstLine func([]byte) error) ([ // copy the slice into buf. r.buf = append(r.buf[:0], trim(line)...) @@ -199,7 +197,7 @@ index fc2590b1cd..fcd1a011ac 100644 r.buf = append(r.buf, trim(line)...) } return r.buf, nil -@@ -507,7 +526,8 @@ func readMIMEHeader(r *Reader, maxMemory, maxHeaders int64) (MIMEHeader, error) +@@ -511,7 +527,8 @@ func readMIMEHeader(r *Reader, maxMemory, maxHeaders int64) (MIMEHeader, error) // The first line cannot start with a leading space. if buf, err := r.R.Peek(1); err == nil && (buf[0] == ' ' || buf[0] == '\t') { @@ -209,7 +207,7 @@ index fc2590b1cd..fcd1a011ac 100644 if err != nil { return m, err } -@@ -515,7 +535,7 @@ func readMIMEHeader(r *Reader, maxMemory, maxHeaders int64) (MIMEHeader, error) +@@ -519,7 +536,7 @@ func readMIMEHeader(r *Reader, maxMemory, maxHeaders int64) (MIMEHeader, error) } for { @@ -218,7 +216,7 @@ index fc2590b1cd..fcd1a011ac 100644 if len(kv) == 0 { return m, err } -@@ -544,7 +564,7 @@ func readMIMEHeader(r *Reader, maxMemory, maxHeaders int64) (MIMEHeader, error) +@@ -540,7 +557,7 @@ func readMIMEHeader(r *Reader, maxMemory, maxHeaders int64) (MIMEHeader, error) maxHeaders-- if maxHeaders < 0 { @@ -227,7 +225,7 @@ index fc2590b1cd..fcd1a011ac 100644 } // Skip initial spaces in value. -@@ -557,9 +577,7 @@ func readMIMEHeader(r *Reader, maxMemory, maxHeaders int64) (MIMEHeader, error) +@@ -553,9 +570,7 @@ func readMIMEHeader(r *Reader, maxMemory, maxHeaders int64) (MIMEHeader, error) } maxMemory -= int64(len(value)) if maxMemory < 0 { @@ -239,7 +237,7 @@ index fc2590b1cd..fcd1a011ac 100644 if vv == nil && len(strs) > 0 { // More than likely this will be a single-element key. diff --git a/src/net/textproto/reader_test.go b/src/net/textproto/reader_test.go -index 696ae406f3..26ff617470 100644 +index f3c372ce03..99ce9bb503 100644 --- a/src/net/textproto/reader_test.go +++ b/src/net/textproto/reader_test.go @@ -36,6 +36,18 @@ func TestReadLine(t *testing.T) { @@ -261,6 +259,3 @@ index 696ae406f3..26ff617470 100644 func TestReadContinuedLine(t *testing.T) { r := reader("line1\nline\n 2\nline3\n") s, err := r.ReadContinuedLine() --- -2.45.1 - From 8bf385ce240b9ce3a4d32c231478341049ed8266 Mon Sep 17 00:00:00 2001 From: David Benoit Date: Mon, 15 Jul 2024 08:59:44 -0400 Subject: [PATCH 3/3] Update patch message with backport info. --- patches/009-net-textproto-mime-multipart-a.patch | 2 ++ 1 file changed, 2 insertions(+) diff --git a/patches/009-net-textproto-mime-multipart-a.patch b/patches/009-net-textproto-mime-multipart-a.patch index c3834f65e9..2fce0ab4eb 100644 --- a/patches/009-net-textproto-mime-multipart-a.patch +++ b/patches/009-net-textproto-mime-multipart-a.patch @@ -19,6 +19,8 @@ Fixes CVE-2023-45290 Fixes #65389 For #65383 +Backported to Go 1.19 by the Golang FIPS Authors. + Change-Id: I7f9264d25752009e95f6b2c80e3d76aaf321d658 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2134435 Reviewed-by: Roland Shoemaker