Skip to content

Commit

Permalink
[release-branch.go1.22] net/textproto, mime/multipart: avoid unbounde…
Browse files Browse the repository at this point in the history
…d 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 golang#65850
For golang#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/+/2174345
Reviewed-by: Carlos Amedee <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/569237
Reviewed-by: Carlos Amedee <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Michael Knyszek <[email protected]>
  • Loading branch information
neild authored and bradfitz committed Mar 5, 2024
1 parent 80f4377 commit 56c3d20
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 15 deletions.
42 changes: 42 additions & 0 deletions src/mime/multipart/formdata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 33 additions & 15 deletions src/net/textproto/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -36,27 +40,33 @@ 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 {
l, more, err := r.R.ReadLine()
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
Expand Down Expand Up @@ -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
}

Expand All @@ -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)
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -507,15 +526,16 @@ 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
}
return m, ProtocolError("malformed MIME header initial line: " + string(line))
}

for {
kv, err := r.readContinuedLineSlice(mustHaveFieldNameColon)
kv, err := r.readContinuedLineSlice(maxMemory, mustHaveFieldNameColon)
if len(kv) == 0 {
return m, err
}
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down
12 changes: 12 additions & 0 deletions src/net/textproto/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 56c3d20

Please sign in to comment.