qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-20 05:54:43 -07:00
Code Releases Activity

net/http: revert overly-strict part of earlier smuggling defense

Browse Source 
The recent https://golang.org/cl/11810 is reportedly a bit too
aggressive.

Apparently some HTTP requests in the wild do contain both a
Transfer-Encoding along with a bogus Content-Length. Instead of
returning a 400 Bad Request error, we should just ignore the
Content-Length like we did before.

Change-Id: I0001be90d09f8293a34f04691f608342875ff5c4
Reviewed-on: https://go-review.googlesource.com/11962
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit is contained in:
Brad Fitzpatrick 2015-07-07 13:19:44 -06:00
parent 7929a0ddfa
commit 143822585e
2 changed files with 44 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
src/net/http/readrequest_test.go
View File

@ -178,6 +178,36 @@ var reqTests = []reqTest{
noError,
},
// Tests chunked body and a bogus Content-Length which should be deleted.
{
"POST / HTTP/1.1\r\n" +
"Host: foo.com\r\n" +
"Transfer-Encoding: chunked\r\n" +
"Content-Length: 9999\r\n\r\n" + // to be removed.
"3\r\nfoo\r\n" +
"3\r\nbar\r\n" +
"0\r\n" +
"\r\n",
&Request{
Method: "POST",
URL: &url.URL{
Path: "/",
},
TransferEncoding: []string{"chunked"},
Proto: "HTTP/1.1",
ProtoMajor: 1,
ProtoMinor: 1,
Header: Header{},
ContentLength: -1,
Host: "foo.com",
RequestURI: "/",
},
"foobar",
noTrailer,
noError,
},
// CONNECT request with domain name:
{
"CONNECT www.google.com:443 HTTP/1.1\r\n\r\n",
@ -399,11 +429,6 @@ var badRequestTests = []struct {
Content-Length: 3
Content-Length: 4
abc`)},
{"smuggle_chunked_and_len", reqBytes(`POST / HTTP/1.1
Transfer-Encoding: chunked
Content-Length: 3
abc`)},
{"smuggle_content_len_head", reqBytes(`HEAD / HTTP/1.1
Host: foo

21
src/net/http/transfer.go
View File

@ -430,7 +430,6 @@ func fixTransferEncoding(isResponse bool, requestMethod string, header Header) (
if !present {
return nil, nil
}
isRequest := !isResponse
delete(header, "Transfer-Encoding")
encodings := strings.Split(raw[0], ",")
@ -458,12 +457,20 @@ func fixTransferEncoding(isResponse bool, requestMethod string, header Header) (
// 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")
}
//
// but also:
// "If a message is received with both a
// Transfer-Encoding and a Content-Length header
// field, the Transfer-Encoding overrides the
// Content-Length. Such a message might indicate an
// attempt to perform request smuggling (Section 9.5)
// or response splitting (Section 9.4) and ought to be
// handled as an error. A sender MUST remove the
// received Content-Length field prior to forwarding
// such a message downstream."
//
// Reportedly, these appear in the wild.
delete(header, "Content-Length")
return te, nil
}