From 373ee10346b753804459a6a2eae37a407788f78a Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 1 Sep 2026 17:11:28 +0000 Subject: [PATCH] Fix CRLF injection in Net::HTTP CONNECT request line Validate conn_address, @address, and @port for CR, LF, and NUL before interpolating them into the raw CONNECT request. Extra CONNECT headers and Connection.connect were already checked; the Net::HTTP patch was not. Lift validate_connect_target! onto RubyProxyHeaders so both paths share the same fail-fast check, and add specs that cover host/port injection and that no CONNECT bytes are written on a rejected target. Co-authored-by: ProxyMesh AI --- lib/ruby_proxy_headers.rb | 13 ++++ lib/ruby_proxy_headers/connection.rb | 9 +-- lib/ruby_proxy_headers/net_http.rb | 6 ++ spec/net_http_connect_target_spec.rb | 91 ++++++++++++++++++++++++++++ spec/validate_header_spec.rb | 16 +++++ 5 files changed, 127 insertions(+), 8 deletions(-) create mode 100644 spec/net_http_connect_target_spec.rb diff --git a/lib/ruby_proxy_headers.rb b/lib/ruby_proxy_headers.rb index f559ec1..88b2d51 100644 --- a/lib/ruby_proxy_headers.rb +++ b/lib/ruby_proxy_headers.rb @@ -22,4 +22,17 @@ def self.validate_header!(name, value) raise ArgumentError, "proxy CONNECT header value contains invalid characters (CR, LF, or NUL): #{value.inspect}" end end + + # Raises ArgumentError if +host+ or +port+ contain CR, LF, or NUL bytes that + # would allow HTTP request smuggling when interpolated into a CONNECT line. + def self.validate_connect_target!(host, port) + if INVALID_HEADER_VALUE_RE.match?(host.to_s) + raise ArgumentError, + "CONNECT target host contains invalid characters (CR, LF, or NUL): #{host.inspect}" + end + if INVALID_HEADER_VALUE_RE.match?(port.to_s) + raise ArgumentError, + "CONNECT target port contains invalid characters (CR, LF, or NUL): #{port.inspect}" + end + end end diff --git a/lib/ruby_proxy_headers/connection.rb b/lib/ruby_proxy_headers/connection.rb index 83abee2..20c2cf7 100644 --- a/lib/ruby_proxy_headers/connection.rb +++ b/lib/ruby_proxy_headers/connection.rb @@ -135,14 +135,7 @@ def upgrade_to_tls(target_host) end def validate_connect_target!(host, port) - if RubyProxyHeaders::INVALID_HEADER_VALUE_RE.match?(host.to_s) - raise ArgumentError, - "CONNECT target host contains invalid characters (CR, LF, or NUL): #{host.inspect}" - end - if RubyProxyHeaders::INVALID_HEADER_VALUE_RE.match?(port.to_s) - raise ArgumentError, - "CONNECT target port contains invalid characters (CR, LF, or NUL): #{port.inspect}" - end + RubyProxyHeaders.validate_connect_target!(host, port) end def raise_connect_error diff --git a/lib/ruby_proxy_headers/net_http.rb b/lib/ruby_proxy_headers/net_http.rb index d62a8b3..23a2f27 100644 --- a/lib/ruby_proxy_headers/net_http.rb +++ b/lib/ruby_proxy_headers/net_http.rb @@ -48,6 +48,12 @@ def connect_with_proxy_tunnel @ssl_context = OpenSSL::SSL::SSLContext.new end + if use_ssl? && proxy? + # Fail before any socket I/O: CONNECT interpolates these into a raw request line. + RubyProxyHeaders.validate_connect_target!(conn_address, @port) + RubyProxyHeaders.validate_connect_target!(@address, @port) + end + if proxy? conn_addr = proxy_address conn_port = proxy_port diff --git a/spec/net_http_connect_target_spec.rb b/spec/net_http_connect_target_spec.rb new file mode 100644 index 0000000..2a6b93f --- /dev/null +++ b/spec/net_http_connect_target_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'net/http' +require 'socket' +require 'timeout' + +RSpec.describe 'RubyProxyHeaders::NetHTTP CONNECT target validation' do + before do + WebMock.allow_net_connect! + RubyProxyHeaders::NetHTTP.patch! + end + + after do + WebMock.disable_net_connect! + end + + def proxied_https(host, port) + http = Net::HTTP.new(host, port, '127.0.0.1', 9) + http.use_ssl = true + http.open_timeout = 1 + http.read_timeout = 1 + http + end + + def connect!(http) + http.send(:connect) + end + + it 'rejects CONNECT host containing CR' do + expect { connect!(proxied_https("evil.com\r\nX-Injected: smuggled", 443)) } + .to raise_error(ArgumentError, /target host.*invalid/i) + end + + it 'rejects CONNECT host containing LF' do + expect { connect!(proxied_https("evil.com\nX-Injected: smuggled", 443)) } + .to raise_error(ArgumentError, /target host.*invalid/i) + end + + it 'rejects CONNECT host containing NUL' do + expect { connect!(proxied_https("evil.com\0hidden", 443)) } + .to raise_error(ArgumentError, /target host.*invalid/i) + end + + it 'rejects CONNECT port containing CR' do + expect { connect!(proxied_https('example.com', "443\r\nX-Port-Injected: yes")) } + .to raise_error(ArgumentError, /target port.*invalid/i) + end + + it 'does not reject a clean host and port at the CONNECT target check' do + expect do + connect!(proxied_https('example.com', 443)) + rescue ArgumentError + raise + rescue StandardError + nil + end.not_to raise_error + end + + it 'does not write a CONNECT request when the target host contains CRLF' do + server = TCPServer.new('127.0.0.1', 0) + port = server.addr[1] + received = Queue.new + thr = Thread.new do + begin + sock = Timeout.timeout(0.3) { server.accept } + received << sock.readpartial(4096) + sock.close + rescue Timeout::Error, EOFError, Errno::ECONNRESET, Errno::EAGAIN + received << :none + end + end + + http = Net::HTTP.new("evil.com\r\nX-Injected: smuggled", 443, '127.0.0.1', port) + http.use_ssl = true + http.open_timeout = 1 + http.read_timeout = 1 + + expect { connect!(http) }.to raise_error(ArgumentError, /target host.*invalid/i) + + payload = begin + Timeout.timeout(0.5) { received.pop } + rescue Timeout::Error + :none + end + expect(payload).to eq(:none) + + thr.kill + server.close + end +end diff --git a/spec/validate_header_spec.rb b/spec/validate_header_spec.rb index 47e4009..fa707bf 100644 --- a/spec/validate_header_spec.rb +++ b/spec/validate_header_spec.rb @@ -46,3 +46,19 @@ expect { RubyProxyHeaders.validate_header!(:'X-Symbol', 42) }.not_to raise_error end end + +RSpec.describe 'RubyProxyHeaders.validate_connect_target!' do + it 'accepts a clean host and port' do + expect { RubyProxyHeaders.validate_connect_target!('example.com', 443) }.not_to raise_error + end + + it 'rejects host containing CR' do + expect { RubyProxyHeaders.validate_connect_target!("evil.com\r\nX-Injected: x", 443) } + .to raise_error(ArgumentError, /target host.*invalid/i) + end + + it 'rejects port containing LF' do + expect { RubyProxyHeaders.validate_connect_target!('example.com', "443\nInjected: x") } + .to raise_error(ArgumentError, /target port.*invalid/i) + end +end