1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
|
From 117ddcb83d7f42d6aa72241240af99ded81118e9 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@golang.org>
Date: Tue, 30 Jun 2015 09:22:41 -0700
Subject: [PATCH] net/textproto: don't treat spaces as hyphens in header keys
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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>
---
src/net/http/header.go | 2 ++
src/net/textproto/reader.go | 36 +++++++++++++++++++++++++++++++++---
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 153b943..d847b13 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 e4b8f6b..91303fe 100644
--- a/src/net/textproto/reader.go
+++ b/src/net/textproto/reader.go
@@ -547,11 +547,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))
}
@@ -565,19 +570,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 6bbd993..8fce7dd 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) {
@@ -194,7 +197,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 {
|