mime/multipart: allow nested boundary with outer boundary+dash prefix

Fixes #46042

Change-Id: Icd243eb12c6e260aeead04710f12340048a0e859
Reviewed-on: https://go-review.googlesource.com/c/go/+/338549
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Trust: Cherry Mui <cherryyz@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>

src/mime/multipart/multipart.go

@@ -264,7 +264,8 @@ func scanUntilBoundary(buf, dashBoundary, nlDashBoundary []byte, total int64, re
 // and the caller has verified already that bytes.HasPrefix(buf, prefix) is true.
 //
 // matchAfterPrefix returns +1 if the buffer does match the boundary,
-// meaning the prefix is followed by a dash, space, tab, cr, nl, or end of input.
+// meaning the prefix is followed by a double dash, space, tab, cr, nl,
+// or end of input.
 // It returns -1 if the buffer definitely does NOT match the boundary,
 // meaning the prefix is followed by some other character.
 // For example, "--foobar" does not match "--foo".
@@ -278,9 +279,25 @@ func matchAfterPrefix(buf, prefix []byte, readErr error) int {
 		return 0
 	}
 	c := buf[len(prefix)]
-	if c == ' ' || c == '\t' || c == '\r' || c == '\n' || c == '-' {
+
+	if c == ' ' || c == '\t' || c == '\r' || c == '\n' {
 		return +1
 	}
+
+	// Try to detect boundaryDash
+	if c == '-' {
+		if len(buf) == len(prefix)+1 {
+			if readErr != nil {
+				// Prefix + "-" does not match
+				return -1
+			}
+			return 0
+		}
+		if buf[len(prefix)+1] == '-' {
+			return +1
+		}
+	}
+
 	return -1
 }
 

src/mime/multipart/multipart_test.go

@@ -291,14 +291,23 @@ func TestLineLimit(t *testing.T) {
 }
 
 func TestMultipartTruncated(t *testing.T) {
-	testBody := `
+	for _, body := range []string{
+		`
 This is a multi-part message.  This line is ignored.
 --MyBoundary
 foo-bar: baz
 
 Oh no, premature EOF!
+`,
 		`
-	body := strings.ReplaceAll(testBody, "\n", "\r\n")
+This is a multi-part message.  This line is ignored.
+--MyBoundary
+foo-bar: baz
+
+Oh no, premature EOF!
+--MyBoundary-`,
+	} {
+		body = strings.ReplaceAll(body, "\n", "\r\n")
 		bodyReader := strings.NewReader(body)
 		r := NewReader(bodyReader, "MyBoundary")
 
@@ -311,6 +320,7 @@ Oh no, premature EOF!
 			t.Fatalf("expected error io.ErrUnexpectedEOF; got %v", err)
 		}
 	}
+}
 
 type slowReader struct {
 	r io.Reader
@@ -751,6 +761,7 @@ html things
 			},
 		},
 	},
+
 	// Issue 12662: Check that we don't consume the leading \r if the peekBuffer
 	// ends in '\r\n--separator-'
 	{
@@ -767,6 +778,7 @@ Content-Type: application/octet-stream
 			},
 		},
 	},
+
 	// Issue 12662: Same test as above with \r\n at the end
 	{
 		name: "peek buffer boundary condition",
@@ -782,6 +794,7 @@ Content-Type: application/octet-stream
 			},
 		},
 	},
+
 	// Issue 12662v2: We want to make sure that for short buffers that end with
 	// '\r\n--separator-' we always consume at least one (valid) symbol from the
 	// peekBuffer
@@ -799,6 +812,7 @@ Content-Type: application/octet-stream
 			},
 		},
 	},
+
 	// Context: https://github.com/camlistore/camlistore/issues/642
 	// If the file contents in the form happens to have a size such as:
 	// size = peekBufferSize - (len("\n--") + len(boundary) + len("\r") + 1), (modulo peekBufferSize)
@@ -832,6 +846,52 @@ val
 		},
 	},
 
+	// Issue 46042; a nested multipart uses the outer separator followed by
+	// a dash.
+	{
+		name: "nested separator prefix is outer separator followed by a dash",
+		sep:  "foo",
+		in: strings.Replace(`--foo
+Content-Type: multipart/alternative; boundary="foo-bar"
+
+--foo-bar
+
+Body
+--foo-bar
+
+Body2
+--foo-bar--
+--foo--`, "\n", "\r\n", -1),
+		want: []headerBody{
+			{textproto.MIMEHeader{"Content-Type": {`multipart/alternative; boundary="foo-bar"`}},
+				strings.Replace(`--foo-bar
+
+Body
+--foo-bar
+
+Body2
+--foo-bar--`, "\n", "\r\n", -1),
+			},
+		},
+	},
+
+	// A nested boundary cannot be the outer separator followed by double dash.
+	{
+		name: "nested separator prefix is outer separator followed by double dash",
+		sep:  "foo",
+		in: strings.Replace(`--foo
+Content-Type: multipart/alternative; boundary="foo--"
+
+--foo--
+
+Body
+
+--foo--`, "\n", "\r\n", -1),
+		want: []headerBody{
+			{textproto.MIMEHeader{"Content-Type": {`multipart/alternative; boundary="foo--"`}}, ""},
+		},
+	},
+
 	roundTripParseTest(),
 }