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
143
144
145
146
|
From 300d9a21583e7cf0149a778a0611e76ff7c6680f Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@golang.org>
Date: Tue, 30 Jun 2015 14:21:15 -0700
Subject: [PATCH] net/http: harden Server against request smuggling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
See RFC 7230.
Thanks to Régis Leroy for the report.
Change-Id: Ic1779bc2180900430d4d7a4938cac04ed73c304c
Reviewed-on: https://go-review.googlesource.com/11810
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
---
src/net/http/readrequest_test.go | 65 ++++++++++++++++++++++++++++++++++++----
src/net/http/transfer.go | 50 ++++++++++++++++++++++++-------
2 files changed, 99 insertions(+), 16 deletions(-)
diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go
index 0cd94eb..3c868bd 100644
--- a/src/net/http/transfer.go
+++ b/src/net/http/transfer.go
@@ -148,6 +148,9 @@ func (t *transferWriter) shouldSendContentLength() bool {
return true
}
if t.ContentLength == 0 && isIdentity(t.TransferEncoding) {
+ if t.Method == "GET" || t.Method == "HEAD" {
+ return false
+ }
return true
}
@@ -317,6 +320,7 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) {
}
case *Request:
t.Header = rr.Header
+ t.RequestMethod = rr.Method
t.ProtoMajor = rr.ProtoMajor
t.ProtoMinor = rr.ProtoMinor
// Transfer semantics for Requests are exactly like those for
@@ -333,7 +337,7 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) {
}
// Transfer encoding, content length
- t.TransferEncoding, err = fixTransferEncoding(t.RequestMethod, t.Header)
+ t.TransferEncoding, err = fixTransferEncoding(isResponse, t.RequestMethod, t.Header)
if err != nil {
return err
}
@@ -421,12 +425,12 @@ func chunked(te []string) bool { return len(te) > 0 && te[0] == "chunked" }
func isIdentity(te []string) bool { return len(te) == 1 && te[0] == "identity" }
// Sanitize transfer encoding
-func fixTransferEncoding(requestMethod string, header Header) ([]string, error) {
+func fixTransferEncoding(isResponse bool, requestMethod string, header Header) ([]string, error) {
raw, present := header["Transfer-Encoding"]
if !present {
return nil, nil
}
-
+ isRequest := !isResponse
delete(header, "Transfer-Encoding")
encodings := strings.Split(raw[0], ",")
@@ -451,10 +455,15 @@ func fixTransferEncoding(requestMethod string, header Header) ([]string, error)
return nil, &badStringError{"too many transfer encodings", strings.Join(te, ",")}
}
if len(te) > 0 {
- // Chunked encoding trumps Content-Length. See RFC 2616
- // Section 4.4. Currently len(te) > 0 implies chunked
- // encoding.
- delete(header, "Content-Length")
+ // RFC 7230 3.3.2 says "A sender MUST NOT send a
+ // Content-Length header field in any message that
+ // contains a Transfer-Encoding header field."
+ if len(header["Content-Length"]) > 0 {
+ if isRequest {
+ return nil, errors.New("http: invalid Content-Length with Transfer-Encoding")
+ }
+ delete(header, "Content-Length")
+ }
return te, nil
}
@@ -465,9 +474,17 @@ func fixTransferEncoding(requestMethod string, header Header) ([]string, error)
// function is not a method, because ultimately it should be shared by
// ReadResponse and ReadRequest.
func fixLength(isResponse bool, status int, requestMethod string, header Header, te []string) (int64, error) {
-
+ contentLens := header["Content-Length"]
+ isRequest := !isResponse
// Logic based on response type or status
if noBodyExpected(requestMethod) {
+ // For HTTP requests, as part of hardening against request
+ // smuggling (RFC 7230), don't allow a Content-Length header for
+ // methods which don't permit bodies. As an exception, allow
+ // exactly one Content-Length header if its value is "0".
+ if isRequest && len(contentLens) > 0 && !(len(contentLens) == 1 && contentLens[0] == "0") {
+ return 0, fmt.Errorf("http: method cannot contain a Content-Length; got %q", contentLens)
+ }
return 0, nil
}
if status/100 == 1 {
@@ -478,13 +495,21 @@ func fixLength(isResponse bool, status int, requestMethod string, header Header,
return 0, nil
}
+ if len(contentLens) > 1 {
+ // harden against HTTP request smuggling. See RFC 7230.
+ return 0, errors.New("http: message cannot contain multiple Content-Length headers")
+ }
+
// Logic based on Transfer-Encoding
if chunked(te) {
return -1, nil
}
// Logic based on Content-Length
- cl := strings.TrimSpace(header.get("Content-Length"))
+ var cl string
+ if len(contentLens) == 1 {
+ cl = strings.TrimSpace(contentLens[0])
+ }
if cl != "" {
n, err := parseContentLength(cl)
if err != nil {
@@ -495,11 +520,14 @@ func fixLength(isResponse bool, status int, requestMethod string, header Header,
header.Del("Content-Length")
}
- if !isResponse && requestMethod == "GET" {
- // RFC 2616 doesn't explicitly permit nor forbid an
+ if !isResponse {
+ // RFC 2616 neither explicitly permits nor forbids an
// entity-body on a GET request so we permit one if
// declared, but we default to 0 here (not -1 below)
// if there's no mention of a body.
+ // Likewise, all other request methods are assumed to have
+ // no body if neither Transfer-Encoding chunked nor a
+ // Content-Length are set.
return 0, nil
}
|
