From ead679d8049ec0643120e4ef1a49a6ef0059ea27 Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 9 Aug 2026 10:38:28 +0200 Subject: [PATCH] Reject CR/LF/NUL in the CONNECT proxy target host The CONNECT handshake concatenates the target host into the request line and Host header with no check for control bytes. normalize/2 percent- decodes the host, so a host carrying %0d%0a arrives as raw CRLF and splits the request sent to the proxy. The WebSocket and WebTransport authority builders already guard against this; the proxy handshake did not. Reject CR/LF/NUL in the target host before building any payload. --- src/hackney_http_connect.erl | 17 ++++++++++++ test/hackney_http_connect_tests.erl | 41 +++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 test/hackney_http_connect_tests.erl diff --git a/src/hackney_http_connect.erl b/src/hackney_http_connect.erl index 4661cf20..e068497a 100644 --- a/src/hackney_http_connect.erl +++ b/src/hackney_http_connect.erl @@ -195,6 +195,23 @@ sockname({Transport, Socket}) -> %% private functions do_handshake(Socket, ProxyTransport, Host, Port, Options) -> + %% The target host is concatenated straight into the CONNECT request line + %% and the Host header. A percent-decoded host can carry raw CR/LF/NUL, which + %% would split the request sent to the proxy. Reject it before building any + %% payload, mirroring the WebSocket/WebTransport authority guards. + case host_has_ctl_bytes(Host) of + true -> + {error, invalid_connect_host}; + false -> + do_handshake_1(Socket, ProxyTransport, Host, Port, Options) + end. + +host_has_ctl_bytes(Host) when is_list(Host) -> + lists:any(fun(C) -> C =:= $\r orelse C =:= $\n orelse C =:= 0 end, Host); +host_has_ctl_bytes(Host) when is_binary(Host) -> + binary:match(Host, [<<"\r">>, <<"\n">>, <<0>>]) =/= nomatch. + +do_handshake_1(Socket, ProxyTransport, Host, Port, Options) -> ProxyUser = proplists:get_value(connect_user, Options), ProxyPass = proplists:get_value(connect_pass, Options, <<>>), ProxyPort = proplists:get_value(connect_port, Options), diff --git a/test/hackney_http_connect_tests.erl b/test/hackney_http_connect_tests.erl new file mode 100644 index 00000000..3cfac1a9 --- /dev/null +++ b/test/hackney_http_connect_tests.erl @@ -0,0 +1,41 @@ +%%% CRLF injection guard for the CONNECT proxy handshake. +%%% +%%% The target host is concatenated into the CONNECT request line and Host +%%% header, so a host carrying CR/LF/NUL (e.g. from a percent-decoded URL) must +%%% be rejected before any payload is built and sent to the proxy. +-module(hackney_http_connect_tests). + +-include_lib("eunit/include/eunit.hrl"). + +crlf_host_rejected_test() -> + assert_rejected("victim.example\r\nX-Injected: yes"). + +lf_host_rejected_test() -> + assert_rejected("victim.example\nX-Injected: yes"). + +nul_host_rejected_test() -> + assert_rejected("victim.example\0"). + +%% Point the handshake at a local listener acting as the proxy: the guard must +%% return {error, invalid_connect_host} and no CONNECT bytes may reach it. +assert_rejected(Host) -> + {ok, LSock} = gen_tcp:listen(0, [binary, {active, false}, + {reuseaddr, true}, {ip, {127, 0, 0, 1}}]), + {ok, LPort} = inet:port(LSock), + Opts = [{connect_host, Host}, + {connect_port, 443}, + {connect_transport, hackney_tcp}, + {proxy_transport, tcp}], + Result = hackney_http_connect:connect("127.0.0.1", LPort, Opts, 1000), + ?assertEqual({error, invalid_connect_host}, Result), + Received = case gen_tcp:accept(LSock, 300) of + {ok, S} -> + R = gen_tcp:recv(S, 0, 100), + gen_tcp:close(S), + R; + {error, timeout} -> + no_connection + end, + gen_tcp:close(LSock), + %% Whatever happened, the proxy must not have seen request bytes. + ?assert(Received =:= no_connection orelse Received =:= {error, closed}).