Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions lib/ruby_proxy_headers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 1 addition & 8 deletions lib/ruby_proxy_headers/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions lib/ruby_proxy_headers/net_http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
91 changes: 91 additions & 0 deletions spec/net_http_connect_target_spec.rb
Original file line number Diff line number Diff line change
@@ -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
16 changes: 16 additions & 0 deletions spec/validate_header_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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