-
Notifications
You must be signed in to change notification settings - Fork 24
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #213 from dbenoit17/go1.20-cve-2023-45290
Backport fix for CVE-2023-45290
- Loading branch information
Showing
1 changed file
with
266 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,266 @@ | ||
From c911efa18163bea9ef152be227b07c5cac0e449d Mon Sep 17 00:00:00 2001 | ||
From: Damien Neil <[email protected]> | ||
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 <[email protected]> | ||
Reviewed-by: Tatiana Bradley <[email protected]> | ||
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2173776 | ||
Reviewed-by: Carlos Amedee <[email protected]> | ||
Reviewed-on: https://go-review.googlesource.com/c/go/+/569240 | ||
Auto-Submit: Michael Knyszek <[email protected]> | ||
LUCI-TryBot-Result: Go LUCI <[email protected]> | ||
Reviewed-by: Carlos Amedee <[email protected]> | ||
--- | ||
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 | ||
|