mime/multipart: percent-encode CR and LF in header values to avoid CRLF injection

When provided with a field or file name containing newlines,
multipart.FileContentDisposition and other header-producing functions
could create an invalid header value.

In some scenarios, this could permit a malicious input to perform
a CRLF injection attack:

  field := "field"
  evilFile := "name\"\r\nEvil-Header: \"evil"
  fmt.Printf("Content-Disposition: %v\r\n", multipart.FileContentDisposition(field, evilFile))
  // Prints:
  // Content-Disposition: form-data; name="field"; filename="name"
  // Evil-Header: "evil"

Percent-endode \r and \n characters in headers, as recommended by
https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#multipart/form-data-encoding-algorithm

The above algorithm also recommends using percent-encoding for quotes,
but preserve the existing backslash-escape behavior for now.
Empirically, browsers understand backslash-escape in attribute values.

Fixes #75557

Change-Id: Ia203df6ef45a098070f3ebb17f9b6cf80c520ed4
Reviewed-on: https://go-review.googlesource.com/c/go/+/706677
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <nsh@golang.org>
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

diff --git a/src/mime/multipart/writer.go b/src/mime/multipart/writer.go
index 8806ab960b8f0ab3a92803393005ac2f4403cb1f..5d140693699ee4c3fd4ee97b6dea969ef39dfac7 100644 (file)
--- a/src/mime/multipart/writer.go
+++ b/src/mime/multipart/writer.go
@@ -125,8 +125,20 @@ func (w *Writer) CreatePart(header textproto.MIMEHeader) (io.Writer, error) {
        return p, nil
 }
 
-var quoteEscaper = strings.NewReplacer("\\", "\\\\", `"`, "\\\"")
+var quoteEscaper = strings.NewReplacer("\\", "\\\\", `"`, "\\\"", "\r", "%0D", "\n", "%0A")
 
+// escapeQuotes escapes special characters in field parameter values.
+//
+// For historical reasons, this uses \ escaping for " and \ characters,
+// and percent encoding for CR and LF.
+//
+// The WhatWG specification for form data encoding suggests that we should
+// use percent encoding for " (%22), and should not escape \.
+// https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#multipart/form-data-encoding-algorithm
+//
+// Empirically, as of the time this comment was written, it is necessary
+// to escape \ characters or else Chrome (and possibly other browsers) will
+// interpet the unescaped \ as an escape.
 func escapeQuotes(s string) string {
        return quoteEscaper.Replace(s)
 }

diff --git a/src/mime/multipart/writer_test.go b/src/mime/multipart/writer_test.go
index 4af6d8c597d4329d6f4bf52d6498acd30fbc7f2f..c234b961080c84d78984d1cc16a2a1ab6c302d00 100644 (file)
--- a/src/mime/multipart/writer_test.go
+++ b/src/mime/multipart/writer_test.go
@@ -184,6 +184,7 @@ func TestFileContentDisposition(t *testing.T) {
                {`somefield`, `somefile"withquotes".txt`, `form-data; name="somefield"; filename="somefile\"withquotes\".txt"`},
                {`somefield\withbackslash`, "somefile.txt", `form-data; name="somefield\\withbackslash"; filename="somefile.txt"`},
                {"somefield", `somefile\withbackslash.txt`, `form-data; name="somefield"; filename="somefile\\withbackslash.txt"`},
+               {"a\rb\nc", "e\rf\ng", `form-data; name="a%0Db%0Ac"; filename="e%0Df%0Ag"`},
        }
        for i, tt := range tests {
                if found := FileContentDisposition(tt.fieldname, tt.filename); found != tt.want {