net/http: Content-Length values are not validated when Transfer-Encoding: chunked is present

[kenballus]

Go version

go version devel go1.23-b8ac61e6e6 Fri Feb 2 23:14:07 2024 +0000 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/root/.cache/go-build'
GOENV='/root/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/root/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/root/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/app/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/app/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.23-b8ac61e6e6 Fri Feb 2 23:14:07 2024 +0000'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1902103395=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Started a web server using net/http, and sent a request with an invalid Content-Length header, as well as a Transfer-Encoding: chunked header. (e.g. GET / HTTP/1.1\r\nHost: a\r\nContent-Length: blahblahblah\r\nTransfer-Encoding: chunked\r\n\r\n0\r\n\r\n)

What did you see happen?

I received a 200 response, as though the invalid Content-Length value were not sent.

What did you expect to see?

I expected to see a 400 response because the Content-Length value was invalid.

[kenballus]

This discrepancy was found using the HTTP Garden.

[bestgopher]

According to https://greenbytes.de/tech/webdav/rfc2616.html#rfc.section.4.4.p.5, if the message does include a non-identity transfer-coding, the Content-Length must be ignored.

[panjf2000]

This is the expected behavior, please check out:

go/src/net/http/transfer.go

Lines 630 to 666 in b8ac61e

	func (t *transferReader) parseTransferEncoding() error {
	raw, present := t.Header["Transfer-Encoding"]
	if !present {
	return nil
	}
	delete(t.Header, "Transfer-Encoding")
	// Issue 12785; ignore Transfer-Encoding on HTTP/1.0 requests.
	if !t.protoAtLeast(1, 1) {
	return nil
	}
	// Like nginx, we only support a single Transfer-Encoding header field, and
	// only if set to "chunked". This is one of the most security sensitive
	// surfaces in HTTP/1.1 due to the risk of request smuggling, so we keep it
	// strict and simple.
	if len(raw) != 1 {
	return &unsupportedTEError{fmt.Sprintf("too many transfer encodings: %q", raw)}
	}
	if !ascii.EqualFold(raw[0], "chunked") {
	return &unsupportedTEError{fmt.Sprintf("unsupported transfer encoding: %q", raw[0])}
	}
	// 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."
	//
	// 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(t.Header, "Content-Length")
	t.Chunked = true

[kenballus]

RFC 9110 is clear that gateway servers must reject messages that contain an invalid Content-Length value:

Likewise, a sender MUST NOT forward a message with a Content-Length header field value that does not match the ABNF above, with one exception: a recipient of a Content-Length header field value consisting of the same decimal value repeated as a comma-separated list (e.g, "Content-Length: 42, 42") MAY either reject the message as invalid or replace that invalid field value with a single instance of the decimal value, since this likely indicates that a duplicate was generated or combined by an upstream message processor.

A gateway server that is implemented on top of net/http has no way of conforming to this rule. Caddy's reverse proxy violates the above rule for this reason.

[kenballus]

To be clear, I'm not saying that valid Content-Length values should be interpreted when a Transfer-Encoding header is present. They should definitely be ignored. What I'm saying is that invalid Content-Length headers should invalidate the request regardless of the presence of a Transfer-Encoding header.

This is what nearly all other HTTP implementations do, including AIOHTTP, Apache httpd, Bun, cherrypy, daphne, Deno, fasthttp, Gunicorn, Hyper, Hypercorn, Jetty, Lighttpd, Mongoose, Nginx, Node.js, LiteSpeed, Passenger, Apache Tomcat, Tornado, OpenWrt uhttpd, Unicorn, Uvicorn, WEBrick, HAProxy, Pound, and Varnish.

[kenballus]

According to https://greenbytes.de/tech/webdav/rfc2616.html#rfc.section.4.4.p.5, if the message does include a non-identity transfer-coding, the Content-Length must be ignored.

RFC 2616 is obsoleted by RFC 7230, which is itself obsoleted by RFC 9112.

The current version of the text that you're referencing is this:

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 11.2) or response splitting (Section 11.1) and ought to be handled as an error. An intermediary that chooses to forward the message MUST first remove the received Content-Length field and process the Transfer-Encoding (as described below) prior to forwarding the message downstream.

Note that it does not say to ignore the Content-Length header; it instead says that an intermediary that chooses to forward a message containing both Content-Length and Transfer-Encoding must remove the Content-Length header. Since it also says that an intermediary MUST not forward requests with invalid Content-Length headers, my original point stands.

[panjf2000]

It is a bit confusing about the statements of Content-Length across these RFCs, and the statement from the latest RFC 9112 still looks ambiguous to me.

[bestgopher]

Note that it does not say to ignore the Content-Length header; it instead says that an intermediary that chooses to forward a message containing both Content-Length and Transfer-Encoding must remove the Content-Length header. Since it also says that an intermediary MUST not forward requests with invalid Content-Length headers, my original point stands.

In my opinion, an intermediary only removes the Content-Length header rather than checks the value of header is valid or not prior to remove it .

[panjf2000]

After reading through the sections of Content-Length in RFC 9110 and 9112, along with browsing the source code in net/http, I find that rejecting requests with invalid Content-Length headers regardless of the presence of a Transfer-Encoding header seems like more rational than ignoring them if that's also how other mainstream HTTP implementations do.

I'll send a CL for this, but I need some comments from @neild on this before we submit it.

[gopherbot]

Change https://go.dev/cl/561075 mentions this issue: net/http: reject requests with invalid Content-Length in Header

[kenballus]

In my opinion, an intermediary only removes the Content-Length header rather than checks the value of header is valid or not prior to remove it .

I agree that the RFCs could be a little clearer about this, but I think the text really doesn't support your claim.

From RFC 9110, section 8.6:

Likewise, a sender MUST NOT forward a message with a Content-Length header field value that does not match the ABNF above, ...

It's not clear from this text alone whether it's saying that the forwarded version of the message must not contain an invalid Content-Length header, or that messages containing invalid Content-Length headers must not be forwarded in any form.

But the rest of the sentence clears up that ambiguity:

... with one exception: a recipient of a Content-Length header field value consisting of the same decimal value repeated as a comma-separated list (e.g, "Content-Length: 42, 42") MAY either reject the message as invalid or replace that invalid field value with a single instance of the decimal value, since this likely indicates that a duplicate was generated or combined by an upstream message processor.

Given that they're making a point to explicitly say that normalization is acceptable when the Content-Length is a comma-separated list of equal values, this implies that normalization is not acceptable when the Content-Length is otherwise invalid.

This is further supported by RFC 9112, section 6.3:

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 11.2) or response splitting (Section 11.1) and ought to be handled as an error. An intermediary that chooses to forward the message MUST first remove the received Content-Length field and process the Transfer-Encoding (as described below) prior to forwarding the message downstream.

(emphasis mine)
This implies that the intermediary must choose to forward the message before before removing the Content-Length field. Since an intermediary MUST NOT forward a message with an invalid Content-Length header, it must be that invalid Content-Length headers invalidate the message, regardless of the presence of the Transfer-Encoding header.

[neild]

RFC 9112 Section 6.1 states:

Early implementations of Transfer-Encoding would occasionally send both a chunked transfer coding for message framing and an estimated Content-Length header field for use by progress bars. This is why Transfer-Encoding is defined as overriding Content-Length, as opposed to them being mutually incompatible.
...
A server MAY reject a request that contains both Content-Length and Transfer-Encoding or process such a request in accordance with the Transfer-Encoding alone. Regardless, the server MUST close the connection after responding to such a request to avoid the potential attacks.

This seems fairly explicit on the valid options when handling a request with both Content-Length and Transfer-Encoding: either reject it outright; or ignore Content-Length, process the message, and close the connection.

[kenballus]

On second thought, I think the current behavior is fine. I was misinterpreting the RFC.