Repository: wojtekmach/req (elixir)

Issue:
`follow_redirects`: do not leak credentials on protocol and port changes
https://curl.se/docs/CVE-2022-27774.html

Pull request:
do not trust different port, scheme
Closes #99.

Patch:
diff --git a/lib/req.ex b/lib/req.ex
index 69e3a15..ca6c8fe 100644
--- a/lib/req.ex
+++ b/lib/req.ex
@@ -402,7 +402,7 @@ defmodule Req do
     * `:follow_redirects` - if set to `false`, disables automatic response redirects. Defaults to `true`.
 
     * `:location_trusted` - by default, authorization credentials are only sent
-      on redirects to the same host. If `:location_trusted` is set to `true`, credentials
+      on redirects with the same host, scheme and port. If `:location_trusted` is set to `true`, credentials
       will be sent to any host.
 
     * `:max_redirects` - the maximum number of redirects, defaults to `10`.
diff --git a/lib/req/steps.ex b/lib/req/steps.ex
index 404bc7a..08488b4 100644
--- a/lib/req/steps.ex
+++ b/lib/req/steps.ex
@@ -759,8 +759,8 @@ defmodule Req.Steps do
   ## Request Options
 
     * `:location_trusted` - by default, authorization credentials are only sent
-      on redirects to the same host. If `:location_trusted` is set to `true`, credentials
-      will be sent to any host.
+      on redirects with the same host, scheme and port. If `:location_trusted` is set
+      to `true`, credentials will be sent to any host.
 
     * `:max_redirects` - the maximum number of redirects, defaults to `10`.
       If the limit is reached, an erorr is raised.
@@ -837,7 +837,8 @@ defmodule Req.Steps do
   defp remove_credentials_if_untrusted(request, true, _), do: request
 
   defp remove_credentials_if_untrusted(request, _, location_url) do
-    if location_url.host == request.url.host do
+    if {location_url.host, location_url.scheme, location_url.port} ==
+         {request.url.host, request.url.scheme, request.url.port} do
       request
     else
       remove_credentials(request)


Test patch:
diff --git a/test/req/steps_test.exs b/test/req/steps_test.exs
index db0c660..3e78516 100644
--- a/test/req/steps_test.exs
+++ b/test/req/steps_test.exs
@@ -552,38 +552,36 @@ defmodule Req.StepsTest do
   end
 
   test "follow_redirects/2: auth - different host" do
-    adapter = fn request ->
-      case request.url.host do
-        "original" ->
-          assert List.keyfind(request.headers, "authorization", 0)
+    adapter = untrusted_redirect_adapter(:host, "trusted", "untrusted")
 
-          response = %Req.Response{
-            status: 301,
-            headers: [{"location", "http://untrusted"}],
-            body: "redirecting"
-          }
+    assert ExUnit.CaptureLog.capture_log(fn ->
+             assert Req.get!("http://trusted",
+                      adapter: adapter,
+                      auth: {"authorization", "credentials"}
+                    ).status == 200
+           end) =~ "[debug] follow_redirects: redirecting to http://untrusted"
+  end
 
-          {request, response}
+  test "follow_redirects/2: auth - different port" do
+    adapter = untrusted_redirect_adapter(:port, 12345, 23456)
 
-        "untrusted" ->
-          refute List.keyfind(request.headers, "authorization", 0)
+    assert ExUnit.CaptureLog.capture_log(fn ->
+             assert Req.get!("http://trusted:12345",
+                      adapter: adapter,
+                      auth: {"authorization", "credentials"}
+                    ).status == 200
+           end) =~ "[debug] follow_redirects: redirecting to http://trusted:23456"
+  end
 
-          response = %Req.Response{
-            status: 200,
-            headers: [],
-            body: "bad things"
-          }
-
-          {request, response}
-      end
-    end
+  test "follow_redirects/2: auth - different scheme" do
+    adapter = untrusted_redirect_adapter(:scheme, "http", "https")
 
     assert ExUnit.CaptureLog.capture_log(fn ->
-             assert Req.get!("http://original",
+             assert Req.get!("http://trusted",
                       adapter: adapter,
                       auth: {"authorization", "credentials"}
                     ).status == 200
-           end) =~ "[debug] follow_redirects: redirecting to http://untrusted"
+           end) =~ "[debug] follow_redirects: redirecting to https://trusted"
   end
 
   test "follow_redirects/1: skip params", c do
@@ -631,6 +629,39 @@ defmodule Req.StepsTest do
     |> Plug.Conn.send_resp(status, "redirecting to #{url}")
   end
 
+  defp untrusted_redirect_adapter(component, original_value, updated_value) do
+    fn request ->
+      case Map.get(request.url, component) do
+        ^original_value ->
+          assert List.keyfind(request.headers, "authorization", 0)
+
+          new_url =
+            request.url
+            |> Map.put(component, updated_value)
+            |> to_string()
+
+          response = %Req.Response{
+            status: 301,
+            headers: [{"location", new_url}],
+            body: "redirecting"
+          }
+
+          {request, response}
+
+        ^updated_value ->
+          refute List.keyfind(request.headers, "authorization", 0)
+
+          response = %Req.Response{
+            status: 200,
+            headers: [],
+            body: "bad things"
+          }
+
+          {request, response}
+      end
+    end
+  end
+
   ## Error steps
 
   @tag :capture_log