276 lines
9.1 KiB
Diff
276 lines
9.1 KiB
Diff
From ad350209f937e05451e46bf55ca8a13f4b24e58e Mon Sep 17 00:00:00 2001
|
|
From: Damien Neil <dneil@google.com>
|
|
Date: Tue, 16 Jan 2024 15:37:52 -0800
|
|
Subject: [PATCH 1/4] [Backport] net/textproto, mime/multipart: avoid unbounded
|
|
read in MIME header
|
|
|
|
Offering: Cloud Core Network
|
|
CVE: CVE-2023-45290
|
|
Reference: https://go-review.googlesource.com/c/go/+/569341
|
|
|
|
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.
|
|
|
|
Note: The upstream does not submit this change to go1.16 according to the rules of MinorReleases.
|
|
Corego2.x are based on go1.16.5. Therefore, it need to submit the change to corego2.x.
|
|
|
|
Edited-by: zhaoshengwei z00581105
|
|
|
|
Fixes CVE-2023-45290
|
|
Fixes #65383
|
|
|
|
Change-Id: I7f9264d25752009e95f6b2c80e3d76aaf321d658
|
|
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2134435
|
|
Reviewed-by: Roland Shoemaker <bracewell@google.com>
|
|
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
|
|
Reviewed-on: https://go-review.googlesource.com/c/go/+/569341
|
|
Reviewed-by: Carlos Amedee <carlos@golang.org>
|
|
Reviewed-by: Damien Neil <dneil@google.com>
|
|
Auto-Submit: Michael Knyszek <mknyszek@google.com>
|
|
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Signed-off-by: Zhao Sheng Wei zhaoshengwei@huawei.com
|
|
---
|
|
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 24de7b8956..1eb301be64 100644
|
|
--- a/src/mime/multipart/formdata_test.go
|
|
+++ b/src/mime/multipart/formdata_test.go
|
|
@@ -407,6 +407,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 d80f378801..b377a7ce5d 100644
|
|
--- a/src/net/textproto/reader.go
|
|
+++ b/src/net/textproto/reader.go
|
|
@@ -17,6 +17,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 {
|
|
@@ -38,13 +42,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) {
|
|
- 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 {
|
|
buf := make([]byte, len(line))
|
|
copy(buf, line)
|
|
@@ -53,7 +57,10 @@ 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 {
|
|
@@ -61,6 +68,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
|
|
@@ -93,7 +103,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
|
|
}
|
|
|
|
@@ -114,7 +124,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 {
|
|
buf := make([]byte, len(line))
|
|
copy(buf, line)
|
|
@@ -127,13 +137,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
|
|
}
|
|
@@ -161,13 +172,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
|
|
@@ -512,7 +531,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
|
|
}
|
|
@@ -520,7 +540,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
|
|
}
|
|
@@ -541,7 +561,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.
|
|
@@ -558,9 +578,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 3ae0de1353..db1ed91bd5 100644
|
|
--- a/src/net/textproto/reader_test.go
|
|
+++ b/src/net/textproto/reader_test.go
|
|
@@ -34,6 +34,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.33.0
|
|
|