Drop request body on cross-origin 307/308 redirects

src/hackney.erl

@@ -1477,7 +1477,14 @@ follow_redirect(ConnPid, Method, Body, WithBody, Options, CurrentURL, RespHeader
       end,
       NewBody = case NewMethod of
         <<"GET">> -> <<>>;
-        _ -> Body
+        _ ->
+          %% A 307/308 keeps the method and body. Do not forward the body to a
+          %% different host unless the caller set location_trusted, mirroring
+          %% the auth/cookie stripping in maybe_strip_auth_on_redirect/3.
+          case redirect_crosses_host(CurrentURL, NewURL, Options) of
+            true -> <<>>;
+            false -> Body
+          end
       end,
       %% Make new request to the redirect URL
       %% Remove old redirect_count and add incremented one
@@ -1570,6 +1577,12 @@ find_last_slash(Bin, Pos) ->
 %% @private Strip sensitive auth options when redirecting to a different host.
 %% This prevents credential leakage per CVE-2018-1000007.
 %% Use location_trusted option to allow forwarding auth to different hosts (like curl's --location-trusted).
+%% @private True when a redirect leaves the original host and the caller has not
+%% opted into location_trusted, i.e. the request body must not be forwarded.
+redirect_crosses_host(CurrentURL, NewURL, Options) ->
+  proplists:get_value(location_trusted, Options, false) =:= false
+    andalso CurrentURL#hackney_url.host =/= NewURL#hackney_url.host.
+
 maybe_strip_auth_on_redirect(CurrentURL, NewURL, Options) ->
   LocationTrusted = proplists:get_value(location_trusted, Options, false),
   case LocationTrusted of

src/hackney_h3.erl

@@ -189,10 +189,16 @@ handle_redirect(Status, RespHeaders, _RespBody, Method, Url, Headers, Body, Opts
             NewUrl = resolve_redirect_url(Location, Url),
             %% Determine new method based on status code
             NewMethod = redirect_method(Status, Method),
-            %% Clear body for POST->GET redirects
+            %% Clear body for POST->GET redirects, and for a 307/308 that
+            %% crosses origin unless the caller set location_trusted (mirrors
+            %% the credential-header strip below).
             NewBody = case NewMethod of
                 get when Method =/= get -> <<>>;
-                _ -> Body
+                _ ->
+                    case redirect_keeps_body(Url, NewUrl, Opts) of
+                        true -> Body;
+                        false -> <<>>
+                    end
             end,
             %% GHSA-h73q: do not forward credential headers to a different
             %% origin unless the caller opted into location_trusted.
@@ -217,6 +223,12 @@ get_redirect_location(Headers) ->
             end
     end.
 
+%% @private Keep the request body on a 307/308 only for a same-origin redirect,
+%% unless the caller trusts the location. Mirrors the credential-header strip.
+redirect_keeps_body(OldUrl, NewUrl, Opts) ->
+    maps:get(location_trusted, Opts, false) =:= true
+        orelse same_origin(OldUrl, NewUrl).
+
 %% @private GHSA-h73q: strip Authorization / Cookie / Proxy-Authorization
 %% before following a redirect to a different origin (scheme, host or port).
 %% location_trusted lets a caller opt back into forwarding them.

test/hackney_redirect_body_tests.erl

@@ -0,0 +1,70 @@
+%%% Request body must not be forwarded to a cross-origin 307/308 redirect
+%%% target unless the caller opts into location_trusted.
+%%%
+%%% localhost and 127.0.0.1 resolve to the same listener but are distinct host
+%%% strings, which is exactly hackney's cross-host test, so one server is enough.
+-module(hackney_redirect_body_tests).
+
+-include_lib("eunit/include/eunit.hrl").
+
+-define(PORT, 9879).
+
+redirect_body_test_() ->
+    {setup, fun setup/0, fun cleanup/1,
+     [{"307 body dropped on cross-origin", fun cross_origin_drops_body/0},
+      {"307 body kept same-origin", fun same_origin_keeps_body/0},
+      {"307 body kept when location_trusted", fun trusted_keeps_body/0}]}.
+
+setup() ->
+    {ok, _} = application:ensure_all_started(hackney),
+    {ok, _} = application:ensure_all_started(cowboy),
+    Dispatch = cowboy_router:compile([{'_', [{"/[...]", test_http_resource, []}]}]),
+    {ok, _} = cowboy:start_clear(test_redirect_body_http, [{port, ?PORT}],
+                                 #{env => #{dispatch => Dispatch}}),
+    ok.
+
+cleanup(_) ->
+    cowboy:stop_listener(test_redirect_body_http),
+    ok.
+
+-define(SECRET, <<"SUPER-SECRET-VALUE-12345">>).
+
+cross_origin_drops_body() ->
+    Body = post_follow(<<"localhost">>, sink_url(<<"127.0.0.1">>), []),
+    ?assertEqual(nomatch, binary:match(Body, ?SECRET)).
+
+same_origin_keeps_body() ->
+    Body = post_follow(<<"localhost">>, sink_url(<<"localhost">>), []),
+    ?assertNotEqual(nomatch, binary:match(Body, ?SECRET)).
+
+trusted_keeps_body() ->
+    Body = post_follow(<<"localhost">>, sink_url(<<"127.0.0.1">>), [location_trusted]),
+    ?assertNotEqual(nomatch, binary:match(Body, ?SECRET)).
+
+sink_url(Host) ->
+    <<"http://", Host/binary, ":", (integer_to_binary(?PORT))/binary, "/post">>.
+
+%% POST to /redirect-to on FromHost, which 307s to SinkUrl (the /post echo).
+post_follow(FromHost, SinkUrl, ExtraOpts) ->
+    Url = <<"http://", FromHost/binary, ":", (integer_to_binary(?PORT))/binary,
+            "/redirect-to?status_code=307&url=", (cow_uri(SinkUrl))/binary>>,
+    Opts = [{follow_redirect, true}, {max_redirect, 3}, with_body | ExtraOpts],
+    {ok, 200, _H, RespBody} =
+        hackney:request(post, Url,
+                        [{<<"Content-Type">>, <<"text/plain">>}], ?SECRET, Opts),
+    RespBody.
+
+cow_uri(Bin) ->
+    list_to_binary(http_uri_encode(binary_to_list(Bin))).
+
+http_uri_encode(Str) ->
+    lists:flatten([encode_char(C) || C <- Str]).
+
+encode_char(C) when C >= $a, C =< $z -> C;
+encode_char(C) when C >= $A, C =< $Z -> C;
+encode_char(C) when C >= $0, C =< $9 -> C;
+encode_char($.) -> $.;
+encode_char($-) -> $-;
+encode_char($/) -> "%2F";
+encode_char($:) -> "%3A";
+encode_char(C) -> io_lib:format("%~2.16.0B", [C]).

test/test_http_resource.erl

@@ -63,8 +63,9 @@ handle_request(<<"GET">>, <<"/status/", CodeBin/binary>>, Req, State) ->
             reply_json(Code, #{<<"status">> => Code}, Req, State)
     end;
 
-%% GET /redirect-to?url=X&status_code=Y - redirect to URL
-handle_request(<<"GET">>, <<"/redirect-to">>, Req, State) ->
+%% <method> /redirect-to?url=X&status_code=Y - redirect to URL (any method, so
+%% 307/308 body-forwarding can be exercised for POST/PUT/PATCH too)
+handle_request(_Method, <<"/redirect-to">>, Req, State) ->
     QS = cowboy_req:parse_qs(Req),
     Url = proplists:get_value(<<"url">>, QS, <<"/">>),
     StatusCode = case proplists:get_value(<<"status_code">>, QS) of