mime: bypassing multi-boundary check via Parameter Value Continuations

[SYM01]

What version of Go are you using (go version)?

$ go version
go version go1.16.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/sym01/Library/Caches/go-build"
GOENV="/Users/sym01/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/sym01/Workspace/Go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/sym01/Workspace/Go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.16.4"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/cr/jbhvnrzx39s46k8btfr1s2_r0000gp/T/go-build52418455=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

https://play.golang.org/p/JLYLXf40j8y

What did you expect to see?

net/http has a secure-by-default implement for multipart/form-data that doesn't allow multi boundaries.

go/src/mime/mediatype.go

Lines 188 to 191 in 507cc34

	if _, exists := pmap[key]; exists {
	// Duplicate parameter name is bogus.
	return "", nil, errors.New("mime: duplicate parameter name")
	}

E.g., It will stop parsing the form with header Content-Type: multipart/form-data;boundary="boundary";boundary="another-boundary".

What did you see instead?

Due to the support of RFC 2231, we can bypass this security check with header Content-Type: multipart/form-data;boundary="boundary";boundary*0="another";boundary*1="-boundary"

This incorrect implementation could be used for bypassing some security mechanisms, such as Web Application Firewall.

[seankhliao]

cc @neild

[neild]

Is there some RFC or other authoritative source indicating that the current implementation is incorrect? RFC 2231 uses Content-Type as an example and makes no indication that the boundary parameter is excluded.

[SYM01]

Yea, the following Content-Type is normal, and should be supported.

Content-Type: message/external-body; access-type=URL;
URL*0="ftp://";
URL*1="cs.utk.edu/pub/moore/bulk-mailer/bulk-mailer.tar"

But how about the following Content-Type ? Which URL should be treated as the right one?

Content-Type: message/external-body; access-type=URL;
URL="ftp://evil.com/path/to/xxx";
URL*0="ftp://";
URL*1="cs.utk.edu/pub/moore/bulk-mailer/bulk-mailer.tar"

RFC 2231 doesn't indicate that which one is right, but Go will always get the second URL.

[neild]

Ah, I get you. The problem is that we aren't detecting duplicate parameter names when one is specified as an RFC 2231 continuation. (Sorry, I misread your initial example and misunderstood the issue.)

[neild]

We have two test cases for mime.ParseMediaType explicitly verifying that when a parameter is present in both RFC 2231 and traditional forms, the RFC 2231 form takes precedence.

These test cases come from:
http://test.greenbytes.de/tech/tc2231/#attfnboth
http://test.greenbytes.de/tech/tc2231/#attfnboth2

RFC 5987 Section 4.2 states:

Header field specifications need to define whether multiple instances
of parameters with identical parmname components are allowed, and how
they should be processed. This specification suggests that a
parameter using the extended syntax takes precedence. This would
allow producers to use both formats without breaking recipients that
do not understand the extended syntax yet.

The test results from the greenbytes.de test cases indicate that most browsers pick the RFC 2231 encoded value.

I think the current behavior is consistent with common practice in interpreting duplicated parameters.

[SYM01]

Yea, I totally agree with you. It's a trade-off between compatibility and security.

This issue is more like a proposal instead of a bug.

[iredmail]

I got similar issue but with real-world email header, sample:

Content-Type: multipart/alternative; boundary="----RTIR0GV26TDW15Q5CVX979LBL6XN18"; boundary="----RTIR0GV26TDW15Q5CVX979LBL6XN18"

There's a proposal but closed without a fix: #28618