diff options
author | Brad Fitzpatrick <bradfitz@golang.org> | 2015-06-30 09:22:41 -0700 |
---|---|---|
committer | Chris Broadfoot <cbro@golang.org> | 2015-09-22 06:39:39 +0000 |
commit | 8f429671248bbcf956fa8a1b7c3a1072285a3b8b (patch) | |
tree | 6e445c2c35c6bc118a55bf768f01ec625576750f | |
parent | e938de22be4ee63d65152024f75d62482d3d69b4 (diff) | |
download | go-8f429671248bbcf956fa8a1b7c3a1072285a3b8b.tar.gz go-8f429671248bbcf956fa8a1b7c3a1072285a3b8b.zip |
[release-branch.go1.4] net/textproto: don't treat spaces as hyphens in header keys
This was originally done in https://codereview.appspot.com/5690059
(Feb 2012) to deal with bad response headers coming back from webcams,
but it presents a potential security problem with HTTP request
smuggling for request headers containing "Content Length" instead of
"Content-Length".
Part of overall HTTP hardening for request smuggling. See RFC 7230.
Thanks to Régis Leroy for the report.
Change-Id: I92b17fb637c9171c5774ea1437979ae2c17ca88a
Reviewed-on: https://go-review.googlesource.com/11772
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/14249
Reviewed-by: Andrew Gerrand <adg@golang.org>
-rw-r--r-- | src/net/http/header.go | 2 | ||||
-rw-r--r-- | src/net/textproto/reader.go | 36 | ||||
-rw-r--r-- | src/net/textproto/reader_test.go | 11 |
3 files changed, 42 insertions, 7 deletions
diff --git a/src/net/http/header.go b/src/net/http/header.go index 153b94370f8..d847b131184 100644 --- a/src/net/http/header.go +++ b/src/net/http/header.go @@ -168,6 +168,8 @@ func (h Header) WriteSubset(w io.Writer, exclude map[string]bool) error { // letter and any letter following a hyphen to upper case; // the rest are converted to lowercase. For example, the // canonical key for "accept-encoding" is "Accept-Encoding". +// If s contains a space or invalid header field bytes, it is +// returned without modifications. func CanonicalHeaderKey(s string) string { return textproto.CanonicalMIMEHeaderKey(s) } // hasToken reports whether token appears with v, ASCII diff --git a/src/net/textproto/reader.go b/src/net/textproto/reader.go index eea9207f252..e10efcdbdcd 100644 --- a/src/net/textproto/reader.go +++ b/src/net/textproto/reader.go @@ -540,11 +540,16 @@ func (r *Reader) upcomingHeaderNewlines() (n int) { // the rest are converted to lowercase. For example, the // canonical key for "accept-encoding" is "Accept-Encoding". // MIME header keys are assumed to be ASCII only. +// If s contains a space or invalid header field bytes, it is +// returned without modifications. func CanonicalMIMEHeaderKey(s string) string { // Quick check for canonical encoding. upper := true for i := 0; i < len(s); i++ { c := s[i] + if !validHeaderFieldByte(c) { + return s + } if upper && 'a' <= c && c <= 'z' { return canonicalMIMEHeaderKey([]byte(s)) } @@ -558,19 +563,44 @@ func CanonicalMIMEHeaderKey(s string) string { const toLower = 'a' - 'A' +// validHeaderFieldByte reports whether b is a valid byte in a header +// field key. This is actually stricter than RFC 7230, which says: +// tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." / +// "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA +// token = 1*tchar +// TODO: revisit in Go 1.6+ and possibly expand this. But note that many +// servers have historically dropped '_' to prevent ambiguities when mapping +// to CGI environment variables. +func validHeaderFieldByte(b byte) bool { + return ('A' <= b && b <= 'Z') || + ('a' <= b && b <= 'z') || + ('0' <= b && b <= '9') || + b == '-' +} + // canonicalMIMEHeaderKey is like CanonicalMIMEHeaderKey but is // allowed to mutate the provided byte slice before returning the // string. +// +// For invalid inputs (if a contains spaces or non-token bytes), a +// is unchanged and a string copy is returned. func canonicalMIMEHeaderKey(a []byte) string { + // See if a looks like a header key. If not, return it unchanged. + for _, c := range a { + if validHeaderFieldByte(c) { + continue + } + // Don't canonicalize. + return string(a) + } + upper := true for i, c := range a { // Canonicalize: first letter upper case // and upper case after each dash. // (Host, User-Agent, If-Modified-Since). // MIME headers are ASCII only, so no Unicode issues. - if c == ' ' { - c = '-' - } else if upper && 'a' <= c && c <= 'z' { + if upper && 'a' <= c && c <= 'z' { c -= toLower } else if !upper && 'A' <= c && c <= 'Z' { c += toLower diff --git a/src/net/textproto/reader_test.go b/src/net/textproto/reader_test.go index cbc0ed183eb..706ab6e1d36 100644 --- a/src/net/textproto/reader_test.go +++ b/src/net/textproto/reader_test.go @@ -24,11 +24,14 @@ var canonicalHeaderKeyTests = []canonicalHeaderKeyTest{ {"uSER-aGENT", "User-Agent"}, {"user-agent", "User-Agent"}, {"USER-AGENT", "User-Agent"}, - {"üser-agenT", "üser-Agent"}, // non-ASCII unchanged + + // Non-ASCII or anything with spaces or non-token chars is unchanged: + {"üser-agenT", "üser-agenT"}, + {"a B", "a B"}, // This caused a panic due to mishandling of a space: - {"C Ontent-Transfer-Encoding", "C-Ontent-Transfer-Encoding"}, - {"foo bar", "Foo-Bar"}, + {"C Ontent-Transfer-Encoding", "C Ontent-Transfer-Encoding"}, + {"foo bar", "foo bar"}, } func TestCanonicalMIMEHeaderKey(t *testing.T) { @@ -185,7 +188,7 @@ func TestReadMIMEHeaderNonCompliant(t *testing.T) { "Foo": {"bar"}, "Content-Language": {"en"}, "Sid": {"0"}, - "Audio-Mode": {"None"}, + "Audio Mode": {"None"}, "Privilege": {"127"}, } if !reflect.DeepEqual(m, want) || err != nil { |