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.

Author: benoitc

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),

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}).