[release-branch.go1.20] mime/multipart: limit memory/inode consumption of ReadForm

Reader.ReadForm is documented as storing "up to maxMemory bytes + 10MB"
in memory. Parsed forms can consume substantially more memory than
this limit, since ReadForm does not account for map entry overhead
and MIME headers.

In addition, while the amount of disk memory consumed by ReadForm can
be constrained by limiting the size of the parsed input, ReadForm will
create one temporary file per form part stored on disk, potentially
consuming a large number of inodes.

Update ReadForm's memory accounting to include part names,
MIME headers, and map entry overhead.

Update ReadForm to store all on-disk file parts in a single
temporary file.

Files returned by FileHeader.Open are documented as having a concrete
type of *os.File when a file is stored on disk. The change to use a
single temporary file for all parts means that this is no longer the
case when a form contains more than a single file part stored on disk.

The previous behavior of storing each file part in a separate disk
file may be reenabled with GODEBUG=multipartfiles=distinct.

Update Reader.NextPart and Reader.NextRawPart to set a 10MiB cap
on the size of MIME headers.

Thanks to Jakob Ackermann (@das7pad) for reporting this issue.

Updates #58006
Fixes #58363
Fixes CVE-2022-41725

Change-Id: Ibd780a6c4c83ac8bcfd3cbe344f042e9940f2eab
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1714276
Reviewed-by: Julie Qiu <[email protected]>
TryBot-Result: Security TryBots <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
(cherry picked from commit 7d0da0029bfbe3228cc5216ced8c7b3184eb517d)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1728950
Reviewed-by: Damien Neil <[email protected]>
Run-TryBot: Roland Shoemaker <[email protected]>
Reviewed-by: Tatiana Bradley <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/468120
Auto-Submit: Michael Pratt <[email protected]>
Run-TryBot: Michael Pratt <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: neild

src/mime/multipart/formdata.go

@@ -7,6 +7,7 @@ package multipart
 import (
 	"bytes"
 	"errors"
+	"internal/godebug"
 	"io"
 	"math"
 	"net/textproto"
@@ -31,25 +32,61 @@ func (r *Reader) ReadForm(maxMemory int64) (*Form, error) {
 	return r.readForm(maxMemory)
 }
 
+var multipartFiles = godebug.New("multipartfiles")
+
 func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
 	form := &Form{make(map[string][]string), make(map[string][]*FileHeader)}
+	var (
+		file    *os.File
+		fileOff int64
+	)
+	numDiskFiles := 0
+	combineFiles := multipartFiles.Value() != "distinct"
 	defer func() {
+		if file != nil {
+			if cerr := file.Close(); err == nil {
+				err = cerr
+			}
+		}
+		if combineFiles && numDiskFiles > 1 {
+			for _, fhs := range form.File {
+				for _, fh := range fhs {
+					fh.tmpshared = true
+				}
+			}
+		}
 		if err != nil {
 			form.RemoveAll()
+			if file != nil {
+				os.Remove(file.Name())
+			}
 		}
 	}()
 
-	// Reserve an additional 10 MB for non-file parts.
-	maxValueBytes := maxMemory + int64(10<<20)
-	if maxValueBytes <= 0 {
+	// maxFileMemoryBytes is the maximum bytes of file data we will store in memory.
+	// Data past this limit is written to disk.
+	// This limit strictly applies to content, not metadata (filenames, MIME headers, etc.),
+	// since metadata is always stored in memory, not disk.
+	//
+	// maxMemoryBytes is the maximum bytes we will store in memory, including file content,
+	// non-file part values, metdata, and map entry overhead.
+	//
+	// We reserve an additional 10 MB in maxMemoryBytes for non-file data.
+	//
+	// The relationship between these parameters, as well as the overly-large and
+	// unconfigurable 10 MB added on to maxMemory, is unfortunate but difficult to change
+	// within the constraints of the API as documented.
+	maxFileMemoryBytes := maxMemory
+	maxMemoryBytes := maxMemory + int64(10<<20)
+	if maxMemoryBytes <= 0 {
 		if maxMemory < 0 {
-			maxValueBytes = 0
+			maxMemoryBytes = 0
 		} else {
-			maxValueBytes = math.MaxInt64
+			maxMemoryBytes = math.MaxInt64
 		}
 	}
 	for {
-		p, err := r.NextPart()
+		p, err := r.nextPart(false, maxMemoryBytes)
 		if err == io.EOF {
 			break
 		}
@@ -63,59 +100,91 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
 		}
 		filename := p.FileName()
 
+		// Multiple values for the same key (one map entry, longer slice) are cheaper
+		// than the same number of values for different keys (many map entries), but
+		// using a consistent per-value cost for overhead is simpler.
+		maxMemoryBytes -= int64(len(name))
+		maxMemoryBytes -= 100 // map overhead
+		if maxMemoryBytes < 0 {
+			// We can't actually take this path, since nextPart would already have
+			// rejected the MIME headers for being too large. Check anyway.
+			return nil, ErrMessageTooLarge
+		}
+
 		var b bytes.Buffer
 
 		if filename == "" {
 			// value, store as string in memory
-			n, err := io.CopyN(&b, p, maxValueBytes+1)
+			n, err := io.CopyN(&b, p, maxMemoryBytes+1)
 			if err != nil && err != io.EOF {
 				return nil, err
 			}
-			maxValueBytes -= n
-			if maxValueBytes < 0 {
+			maxMemoryBytes -= n
+			if maxMemoryBytes < 0 {
 				return nil, ErrMessageTooLarge
 			}
 			form.Value[name] = append(form.Value[name], b.String())
 			continue
 		}
 
 		// file, store in memory or on disk
+		maxMemoryBytes -= mimeHeaderSize(p.Header)
+		if maxMemoryBytes < 0 {
+			return nil, ErrMessageTooLarge
+		}
 		fh := &FileHeader{
 			Filename: filename,
 			Header:   p.Header,
 		}
-		n, err := io.CopyN(&b, p, maxMemory+1)
+		n, err := io.CopyN(&b, p, maxFileMemoryBytes+1)
 		if err != nil && err != io.EOF {
 			return nil, err
 		}
-		if n > maxMemory {
-			// too big, write to disk and flush buffer
-			file, err := os.CreateTemp("", "multipart-")
-			if err != nil {
-				return nil, err
+		if n > maxFileMemoryBytes {
+			if file == nil {
+				file, err = os.CreateTemp(r.tempDir, "multipart-")
+				if err != nil {
+					return nil, err
+				}
 			}
+			numDiskFiles++
 			size, err := io.Copy(file, io.MultiReader(&b, p))
-			if cerr := file.Close(); err == nil {
-				err = cerr
-			}
 			if err != nil {
-				os.Remove(file.Name())
 				return nil, err
 			}
 			fh.tmpfile = file.Name()
 			fh.Size = size
+			fh.tmpoff = fileOff
+			fileOff += size
+			if !combineFiles {
+				if err := file.Close(); err != nil {
+					return nil, err
+				}
+				file = nil
+			}
 		} else {
 			fh.content = b.Bytes()
 			fh.Size = int64(len(fh.content))
-			maxMemory -= n
-			maxValueBytes -= n
+			maxFileMemoryBytes -= n
+			maxMemoryBytes -= n
 		}
 		form.File[name] = append(form.File[name], fh)
 	}
 
 	return form, nil
 }
 
+func mimeHeaderSize(h textproto.MIMEHeader) (size int64) {
+	for k, vs := range h {
+		size += int64(len(k))
+		size += 100 // map entry overhead
+		for _, v := range vs {
+			size += int64(len(v))
+		}
+	}
+	return size
+}
+
 // Form is a parsed multipart form.
 // Its File parts are stored either in memory or on disk,
 // and are accessible via the *FileHeader's Open method.
@@ -133,7 +202,7 @@ func (f *Form) RemoveAll() error {
 		for _, fh := range fhs {
 			if fh.tmpfile != "" {
 				e := os.Remove(fh.tmpfile)
-				if e != nil && err == nil {
+				if e != nil && !errors.Is(e, os.ErrNotExist) && err == nil {
 					err = e
 				}
 			}
@@ -148,15 +217,25 @@ type FileHeader struct {
 	Header   textproto.MIMEHeader
 	Size     int64
 
-	content []byte
-	tmpfile string
+	content   []byte
+	tmpfile   string
+	tmpoff    int64
+	tmpshared bool
 }
 
 // Open opens and returns the FileHeader's associated File.
 func (fh *FileHeader) Open() (File, error) {
 	if b := fh.content; b != nil {
 		r := io.NewSectionReader(bytes.NewReader(b), 0, int64(len(b)))
-		return sectionReadCloser{r}, nil
+		return sectionReadCloser{r, nil}, nil
+	}
+	if fh.tmpshared {
+		f, err := os.Open(fh.tmpfile)
+		if err != nil {
+			return nil, err
+		}
+		r := io.NewSectionReader(f, fh.tmpoff, fh.Size)
+		return sectionReadCloser{r, f}, nil
 	}
 	return os.Open(fh.tmpfile)
 }
@@ -175,8 +254,12 @@ type File interface {
 
 type sectionReadCloser struct {
 	*io.SectionReader
+	io.Closer
 }
 
 func (rc sectionReadCloser) Close() error {
+	if rc.Closer != nil {
+		return rc.Closer.Close()
+	}
 	return nil
 }