Reject HTTP/1 requests with both content-length and transfer-encoding

The previous behavior was to accept them and drop the
content-length header as per the RFC recommendation.
But since this behavior is not normal it is safer to
just reject such requests than risk security issues.

2 files changed, 19 insertions, 15 deletions

diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl
index bba2108..70f8268 100644
--- a/src/cowboy_http.erl
+++ b/src/cowboy_http.erl
@@ -765,39 +765,42 @@ default_port(_) -> 80.
 request(Buffer, State0=#state{ref=Ref, transport=Transport, peer=Peer, sock=Sock, cert=Cert,
 		proxy_header=ProxyHeader, in_streamid=StreamID, in_state=
 			PS=#ps_header{method=Method, path=Path, qs=Qs, version=Version}},
-		Headers0, Host, Port) ->
+		Headers, Host, Port) ->
 	Scheme = case Transport:secure() of
 		true -> <<"https">>;
 		false -> <<"http">>
 	end,
-	{Headers, HasBody, BodyLength, TDecodeFun, TDecodeState} = case Headers0 of
+	{HasBody, BodyLength, TDecodeFun, TDecodeState} = case Headers of
+		#{<<"transfer-encoding">> := _, <<"content-length">> := _} ->
+			error_terminate(400, State0#state{in_state=PS#ps_header{headers=Headers}},
+				{stream_error, protocol_error,
+					'The request had both transfer-encoding and content-length headers. (RFC7230 3.3.3)'});
 		#{<<"transfer-encoding">> := TransferEncoding0} ->
 			try cow_http_hd:parse_transfer_encoding(TransferEncoding0) of
 				[<<"chunked">>] ->
-					{maps:remove(<<"content-length">>, Headers0),
-						true, undefined, fun cow_http_te:stream_chunked/2, {0, 0}};
+					{true, undefined, fun cow_http_te:stream_chunked/2, {0, 0}};
 				_ ->
-					error_terminate(400, State0#state{in_state=PS#ps_header{headers=Headers0}},
+					error_terminate(400, State0#state{in_state=PS#ps_header{headers=Headers}},
 						{stream_error, protocol_error,
 							'Cowboy only supports transfer-encoding: chunked. (RFC7230 3.3.1)'})
 			catch _:_ ->
-				error_terminate(400, State0#state{in_state=PS#ps_header{headers=Headers0}},
+				error_terminate(400, State0#state{in_state=PS#ps_header{headers=Headers}},
 					{stream_error, protocol_error,
 						'The transfer-encoding header is invalid. (RFC7230 3.3.1)'})
 			end;
 		#{<<"content-length">> := <<"0">>} ->
-			{Headers0, false, 0, undefined, undefined};
+			{false, 0, undefined, undefined};
 		#{<<"content-length">> := BinLength} ->
 			Length = try
 				cow_http_hd:parse_content_length(BinLength)
 			catch _:_ ->
-				error_terminate(400, State0#state{in_state=PS#ps_header{headers=Headers0}},
+				error_terminate(400, State0#state{in_state=PS#ps_header{headers=Headers}},
 					{stream_error, protocol_error,
 						'The content-length header is invalid. (RFC7230 3.3.2)'})
 			end,
-			{Headers0, true, Length, fun cow_http_te:stream_identity/2, {0, Length}};
+			{true, Length, fun cow_http_te:stream_identity/2, {0, Length}};
 		_ ->
-			{Headers0, false, 0, undefined, undefined}
+			{false, 0, undefined, undefined}
 	end,
 	Req0 = #{
 		ref => Ref,
diff --git a/test/rfc7230_SUITE.erl b/test/rfc7230_SUITE.erl
index 50a6c71..64d9ce4 100644
--- a/test/rfc7230_SUITE.erl
+++ b/test/rfc7230_SUITE.erl
@@ -1144,18 +1144,19 @@ reject_invalid_content_length(Config) ->
 %with a message body too large must be rejected with a 413 status
 %code and the closing of the connection. (RFC7230 3.3.2)
 
-ignore_content_length_when_transfer_encoding(Config) ->
+reject_when_both_content_length_and_transfer_encoding(Config) ->
 	doc("When a message includes both transfer-encoding and content-length "
-		"headers, the content-length header must be removed before processing "
-		"the request. (RFC7230 3.3.3)"),
-	#{code := 200, body := <<"Hello world!">>} = do_raw(Config, [
+		"headers, the message may be an attempt at request smuggling. It "
+		"must be rejected with a 400 status code and the closing of the "
+		"connection. (RFC7230 3.3.3)"),
+	#{code := 400, client := Client} = do_raw(Config, [
 		"POST /echo/read_body HTTP/1.1\r\n"
 		"Host: localhost\r\n"
 		"Transfer-encoding: chunked\r\n"
 		"Content-length: 12\r\n"
 		"\r\n"
 		"6\r\nHello \r\n5\r\nworld\r\n1\r\n!\r\n0\r\n\r\n"]),
-	ok.
+	{error, closed} = raw_recv(Client, 0, 1000).
 
 %socket_error_while_reading_body(Config) ->
 %If a socket error occurs while reading the body the server