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 #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>

diff --git a/src/mime/multipart/formdata_test.go b/src/mime/multipart/formdata_test.go
index d422729c96ab4f480d5cb24650b3d15e61dd27a1..bfa9f683825855e45548e731e6dfbeb0336241cf 100644 (file)
--- a/src/mime/multipart/formdata_test.go
+++ b/src/mime/multipart/formdata_test.go
@@ -452,6 +452,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 ee7eb0200bf4e5497547981a7b221c6571594ba1..1a8145355955efc2a503167bb2de8f5ba8a3670b 100644 (file)
--- 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 [Reader.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 [Reader.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
                }
@@ -537,7 +557,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.
@@ -550,9 +570,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 c9c0a98ea4452d1d601bcfc46c49a4fc3051bb4d..f794879bd739fc14288ce36a39b9d56d4fb2ff2d 100644 (file)
--- 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()