CVE-2023-38697: Inconsistent Interpretation of HTTP Requests ('HTTP Request/Response Smuggling') (CWE-444)
value: protocol-http1 provides a low-level implementation of the HTTP/1 protocol. RFC 9112 Section 7.1 defined the format of chunk size, chunk data and chunk extension. The value of Content-Length header should be a string of 0-9 digits, the chunk size should be a string of hex digits and should split from chunk data using CRLF, and the chunk extension shouldn't contain any invisible character. However, Falcon has following behaviors while disobey the corresponding RFCs: accepting Content-Length header values that have `+` prefix, accepting Content-Length header values that written in hexadecimal with `0x` prefix, accepting `0x` and `+` prefixed chunk size, and accepting LF in chunk extension. This behavior can lead to desync when forwarding through multiple HTTP parsers, potentially results in HTTP request smuggling and firewall bypassing. This issue is fixed in `protocol-http1` v0.15.1. There are no known workarounds.
CWE description: The product acts as an intermediary HTTP agent
         (such as a proxy or firewall) in the data flow between two
         entities such as a client and server, but it does not
         interpret malformed HTTP requests or responses in ways that
         are consistent with how the messages will be processed by
         those entities that are at the ultimate destination.
CVSS v3 base score: 5.3
Published: 2023-08-04T18:15Z
Repository: https://github.com/socketry/protocol-http1
Language: Ruby
Files changed: lib/protocol/http1/body/chunked.rb, lib/protocol/http1/connection.rb, test/protocol/http1/connection.rb, test/protocol/http1/connection/bad.rb
Commit date: 2023-07-30 12:46:58 +1200
Commit message: Strict validation of content length and chunk length. (#20)


Diff: diff --git a/lib/protocol/http1/body/chunked.rb b/lib/protocol/http1/body/chunked.rb
index 9a02b54..c31177b 100644
--- a/lib/protocol/http1/body/chunked.rb
+++ b/lib/protocol/http1/body/chunked.rb
@@ -33,16 +33,24 @@ module Protocol
 					end
 					
 					super
 				end
 				
+				VALID_CHUNK_LENGTH = /\A[0-9a-fA-F]+\z/
+				
 				# Follows the procedure outlined in https://tools.ietf.org/html/rfc7230#section-4.1.3
 				def read
 					return nil if @finished
 					
+					length, extensions = read_line.split(";", 2)
+					
+					unless length =~ VALID_CHUNK_LENGTH
+						raise BadRequest, "Invalid chunk length: #{length.dump}"
+					end
+					
 					# It is possible this line contains chunk extension, so we use `to_i` to only consider the initial integral part:
-					length = read_line.to_i(16)
+					length = Integer(length, 16)
 					
 					if length == 0
 						@finished = true
 						
 						read_trailer
diff --git a/lib/protocol/http1/connection.rb b/lib/protocol/http1/connection.rb
index 26dcb5a..045a8ec 100644
--- a/lib/protocol/http1/connection.rb
+++ b/lib/protocol/http1/connection.rb
@@ -396,14 +396,16 @@ module Protocol
 			end
 			
 			HEAD = "HEAD"
 			CONNECT = "CONNECT"
 			
+			VALID_CONTENT_LENGTH = /\A\d+\z/
+			
 			def extract_content_length(headers)
 				if content_length = headers.delete(CONTENT_LENGTH)
-					if length = Integer(content_length, exception: false) and length >= 0
-						yield length
+					if content_length =~ VALID_CONTENT_LENGTH
+						yield Integer(content_length, 10)
 					else
 						raise BadRequest, "Invalid content length: #{content_length}"
 					end
 				end
 			end
diff --git a/test/protocol/http1/connection.rb b/test/protocol/http1/connection.rb
index 6cd134b..081fa1b 100644
--- a/test/protocol/http1/connection.rb
+++ b/test/protocol/http1/connection.rb
@@ -206,19 +206,19 @@ describe Protocol::HTTP1::Connection do
 			end
 		end
 		
 		with "HEAD" do
 			it "can read length of head response" do
-				body = client.read_response_body("HEAD", 200, {'content-length' => 3773})
+				body = client.read_response_body("HEAD", 200, {'content-length' => '3773'})
 				
 				expect(body).to be_a ::Protocol::HTTP::Body::Head
 				expect(body.length).to be == 3773
 				expect(body.read).to be_nil
 			end
 			
 			it "ignores zero length body" do
-				body = client.read_response_body("HEAD", 200, {'content-length' => 0})
+				body = client.read_response_body("HEAD", 200, {'content-length' => '0'})
 				
 				expect(body).to be_nil
 			end
 			
 			it "raises error if content-length is invalid" do
diff --git a/test/protocol/http1/connection/bad.rb b/test/protocol/http1/connection/bad.rb
new file mode 100644
index 0000000..be595f9
--- /dev/null
+++ b/test/protocol/http1/connection/bad.rb
@@ -0,0 +1,125 @@
+# frozen_string_literal: true
+
+# Released under the MIT License.
+# Copyright, 2019-2023, by Samuel Williams.
+
+require 'protocol/http1/connection'
+require 'connection_context'
+
+describe Protocol::HTTP1::Connection do
+	include_context ConnectionContext
+	
+	def before
+		super
+		
+		client.stream.write(input)
+		client.stream.close
+	end
+	
+	with "invalid hexadecimal content-length" do
+		def input
+			<<~HTTP.gsub("\n", "\r\n")
+			POST / HTTP/1.1
+			Host: a.com
+			Content-Length: 0x10
+			Connection: close
+			
+			0123456789abcdef
+			HTTP
+		end
+		
+		it "should fail to parse the request body" do
+			expect do
+				server.read_request
+			end.to raise_exception(Protocol::HTTP1::BadRequest)
+		end
+	end
+	
+	with "invalid +integer content-length" do
+		def input
+			<<~HTTP.gsub("\n", "\r\n")
+			POST / HTTP/1.1
+			Host: a.com
+			Content-Length: +16
+			Connection: close
+			
+			0123456789abcdef
+			HTTP
+		end
+		
+		it "should fail to parse the request body" do
+			expect do
+				server.read_request
+			end.to raise_exception(Protocol::HTTP1::BadRequest)
+		end
+	end
+	
+	with "invalid -integer content-length" do
+		def input
+			<<~HTTP.gsub("\n", "\r\n")
+			POST / HTTP/1.1
+			Host: a.com
+			Content-Length: -16
+			Connection: close
+			
+			0123456789abcdef
+			HTTP
+		end
+		
+		it "should fail to parse the request body" do
+			expect do
+				server.read_request
+			end.to raise_exception(Protocol::HTTP1::BadRequest)
+		end
+	end
+	
+	with "invalid hexidecimal chunk size" do
+		def input
+			<<~HTTP.gsub("\n", "\r\n")
+			POST / HTTP/1.1
+			Host: a.com
+			Transfer-Encoding: chunked
+			Connection: close
+			
+			0x10
+			0123456789abcdef
+			0
+			HTTP
+		end
+		
+		it "should fail to parse the request body" do
+			authority, method, target, version, headers, body = server.read_request
+			
+			expect(body).to be_a(Protocol::HTTP1::Body::Chunked)
+			
+			expect do
+				body.read
+			end.to raise_exception(Protocol::HTTP1::BadRequest)
+		end
+	end
+	
+	with "invalid +integer chunk size" do
+		def input
+			<<~HTTP.gsub("\n", "\r\n")
+			POST / HTTP/1.1
+			Host: a.com
+			Transfer-Encoding: chunked
+			Connection: close
+			
+			+10
+			0123456789abcdef
+			0
+			HTTP
+		end
+		
+		it "should fail to parse the request body" do
+			authority, method, target, version, headers, body = server.read_request
+			
+			expect(body).to be_a(Protocol::HTTP1::Body::Chunked)
+			
+			expect do
+				body.read
+			end.to raise_exception(Protocol::HTTP1::BadRequest)
+		end
+	end
+end

Vulnerable code: 					length = read_line.to_i(16)
					if length = Integer(content_length, exception: false) and length >= 0
						yield length
				body = client.read_response_body("HEAD", 200, {'content-length' => 3773})
				body = client.read_response_body("HEAD", 200, {'content-length' => 0})
Fixed code: 				VALID_CHUNK_LENGTH = /\A[0-9a-fA-F]+\z/
				
					length, extensions = read_line.split(";", 2)
					
					unless length =~ VALID_CHUNK_LENGTH
						raise BadRequest, "Invalid chunk length: #{length.dump}"
					end
					
					length = Integer(length, 16)
			VALID_CONTENT_LENGTH = /\A\d+\z/
			
					if content_length =~ VALID_CONTENT_LENGTH
						yield Integer(content_length, 10)
				body = client.read_response_body("HEAD", 200, {'content-length' => '3773'})
				body = client.read_response_body("HEAD", 200, {'content-length' => '0'})
# frozen_string_literal: true

# Released under the MIT License.
# Copyright, 2019-2023, by Samuel Williams.

require 'protocol/http1/connection'
require 'connection_context'

describe Protocol::HTTP1::Connection do
	include_context ConnectionContext
	
	def before
		super
		
		client.stream.write(input)
		client.stream.close
	end
	
	with "invalid hexadecimal content-length" do
		def input
			<<~HTTP.gsub("\n", "\r\n")
			POST / HTTP/1.1
			Host: a.com
			Content-Length: 0x10
			Connection: close
			
			0123456789abcdef
			HTTP
		end
		
		it "should fail to parse the request body" do
			expect do
				server.read_request
			end.to raise_exception(Protocol::HTTP1::BadRequest)
		end
	end
	
	with "invalid +integer content-length" do
		def input
			<<~HTTP.gsub("\n", "\r\n")
			POST / HTTP/1.1
			Host: a.com
			Content-Length: +16
			Connection: close
			
			0123456789abcdef
			HTTP
		end
		
		it "should fail to parse the request body" do
			expect do
				server.read_request
			end.to raise_exception(Protocol::HTTP1::BadRequest)
		end
	end
	
	with "invalid -integer content-length" do
		def input
			<<~HTTP.gsub("\n", "\r\n")
			POST / HTTP/1.1
			Host: a.com
			Content-Length: -16
			Connection: close
			
			0123456789abcdef
			HTTP
		end
		
		it "should fail to parse the request body" do
			expect do
				server.read_request
			end.to raise_exception(Protocol::HTTP1::BadRequest)
		end
	end
	
	with "invalid hexidecimal chunk size" do
		def input
			<<~HTTP.gsub("\n", "\r\n")
			POST / HTTP/1.1
			Host: a.com
			Transfer-Encoding: chunked
			Connection: close
			
			0x10
			0123456789abcdef
			0
			HTTP
		end
		
		it "should fail to parse the request body" do
			authority, method, target, version, headers, body = server.read_request
			
			expect(body).to be_a(Protocol::HTTP1::Body::Chunked)
			
			expect do
				body.read
			end.to raise_exception(Protocol::HTTP1::BadRequest)
		end
	end
	
	with "invalid +integer chunk size" do
		def input
			<<~HTTP.gsub("\n", "\r\n")
			POST / HTTP/1.1
			Host: a.com
			Transfer-Encoding: chunked
			Connection: close
			
			+10
			0123456789abcdef
			0
			HTTP
		end
		
		it "should fail to parse the request body" do
			authority, method, target, version, headers, body = server.read_request
			
			expect(body).to be_a(Protocol::HTTP1::Body::Chunked)
			
			expect do
				body.read
			end.to raise_exception(Protocol::HTTP1::BadRequest)
		end
	end
end