CVE-2015-5739: Inconsistent Interpretation of HTTP Requests ('HTTP Request/Response Smuggling') (CWE-444)
value: The net/http library in net/textproto/reader.go in Go before 1.4.3 does not properly parse HTTP header keys, which allows remote attackers to conduct HTTP request smuggling attacks via a space instead of a hyphen, as demonstrated by "Content Length" instead of "Content-Length."
CWE description: The product acts as an intermediary HTTP agent
         (such as a proxy or firewall) in the data flow between two
         entities such as a client and server, but it does not
         interpret malformed HTTP requests or responses in ways that
         are consistent with how the messages will be processed by
         those entities that are at the ultimate destination.
CVSS v3 base score: 9.8
Published: 2017-10-18T20:29Z
Repository: https://github.com/golang/go
Language: Go
Files changed: src/net/http/header.go, src/net/textproto/reader.go, src/net/textproto/reader_test.go
Commit date: 2015-06-30 17:59:02 +0000
Commit message: 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>

Diff: diff --git a/src/net/http/header.go b/src/net/http/header.go
index 153b94370..d847b1311 100644
--- a/src/net/http/header.go
+++ b/src/net/http/header.go
@@ -166,10 +166,12 @@ func (h Header) WriteSubset(w io.Writer, exclude map[string]bool) error {
 // CanonicalHeaderKey returns the canonical format of the
 // header key s.  The canonicalization converts the first
 // 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
 // case-insensitive, with space or comma boundaries.
 // token must be all lowercase.
diff --git a/src/net/textproto/reader.go b/src/net/textproto/reader.go
index e4b8f6bb9..91303fec6 100644
--- a/src/net/textproto/reader.go
+++ b/src/net/textproto/reader.go
@@ -545,15 +545,20 @@ func (r *Reader) upcomingHeaderNewlines() (n int) {
 // MIME header key s.  The canonicalization converts the first
 // 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".
 // 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))
 		}
 		if !upper && 'A' <= c && c <= 'Z' {
 			return canonicalMIMEHeaderKey([]byte(s))
@@ -563,23 +568,48 @@ func CanonicalMIMEHeaderKey(s string) string {
 	return s
 }
 
 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
 		}
 		a[i] = c
diff --git a/src/net/textproto/reader_test.go b/src/net/textproto/reader_test.go
index 6bbd993b8..8fce7ddeb 100644
--- a/src/net/textproto/reader_test.go
+++ b/src/net/textproto/reader_test.go
@@ -22,15 +22,18 @@ var canonicalHeaderKeyTests = []canonicalHeaderKeyTest{
 	{"a-1-c", "A-1-C"},
 	{"User-Agent", "User-Agent"},
 	{"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) {
 	for _, tt := range canonicalHeaderKeyTests {
 		if s := CanonicalMIMEHeaderKey(tt.in); s != tt.out {
@@ -192,11 +195,11 @@ func TestReadMIMEHeaderNonCompliant(t *testing.T) {
 	m, err := r.ReadMIMEHeader()
 	want := MIMEHeader{
 		"Foo":              {"bar"},
 		"Content-Language": {"en"},
 		"Sid":              {"0"},
-		"Audio-Mode":       {"None"},
+		"Audio Mode":       {"None"},
 		"Privilege":        {"127"},
 	}
 	if !reflect.DeepEqual(m, want) || err != nil {
 		t.Fatalf("ReadMIMEHeader =\n%v, %v; want:\n%v", m, err, want)
 	}

Vulnerable code: 		if c == ' ' {
			c = '-'
		} else if upper && 'a' <= c && c <= 'z' {
	{"üser-agenT", "üser-Agent"}, // non-ASCII unchanged
	{"C Ontent-Transfer-Encoding", "C-Ontent-Transfer-Encoding"},
	{"foo bar", "Foo-Bar"},
		"Audio-Mode":       {"None"},
Fixed code: // If s contains a space or invalid header field bytes, it is
// returned without modifications.
// If s contains a space or invalid header field bytes, it is
// returned without modifications.
		if !validHeaderFieldByte(c) {
			return s
		}
// 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 == '-'
}

//
// For invalid inputs (if a contains spaces or non-token bytes), a
// is unchanged and a string copy is returned.
	// 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)
	}

		if upper && 'a' <= c && c <= 'z' {

	// Non-ASCII or anything with spaces or non-token chars is unchanged:
	{"üser-agenT", "üser-agenT"},
	{"a B", "a B"},
	{"C Ontent-Transfer-Encoding", "C Ontent-Transfer-Encoding"},
	{"foo bar", "foo bar"},
		"Audio Mode":       {"None"},