From d5b0720596b70240bb559a2084d3c3cdc76fe7b2 Mon Sep 17 00:00:00 2001
From: Oneric <oneric@oneric.stub>
Date: Sat, 13 Jul 2024 06:54:45 +0200
Subject: [PATCH] Allow cross-domain redirects on AP requests

Since we now remember the final location redirects lead to
and use it for all further checks since
3e134b07fa4e382f1f4cfdbe90e74f8e73336a4e, these redirects
can no longer be exploited to serve counterfeit objects.

This fixes:
 - display URLs from independent webapp clients
   redirecting to the canonical domain
 - Peertube display URLs for remote content
   (acting like the above)
---
 lib/pleroma/object/fetcher.ex        | 18 +---------------
 test/pleroma/object/fetcher_test.exs | 32 ++++++++++++++++++++++++----
 2 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex
index 7f9a922aa..2cecfec67 100644
--- a/lib/pleroma/object/fetcher.ex
+++ b/lib/pleroma/object/fetcher.ex
@@ -317,7 +317,7 @@ defmodule Pleroma.Object.Fetcher do
 
       {:containment, reason} ->
         log_fetch_error(id, reason)
-        {:error, reason}
+        {:error, {:containment, reason}}
 
       {:error, e} ->
         {:error, e}
@@ -330,22 +330,10 @@ defmodule Pleroma.Object.Fetcher do
   def fetch_and_contain_remote_object_from_id(_id, _is_ap_id),
     do: {:error, :invalid_id}
 
-  defp check_crossdomain_redirect(final_host, original_url)
-
   # HOPEFULLY TEMPORARY
   # Basically none of our Tesla mocks in tests set the (supposed to
   # exist for Tesla proper) url parameter for their responses
   # causing almost every fetch in test to fail otherwise
-  if @mix_env == :test do
-    defp check_crossdomain_redirect(nil, _) do
-      {:cross_domain_redirect, false}
-    end
-  end
-
-  defp check_crossdomain_redirect(final_host, original_url) do
-    {:cross_domain_redirect, final_host != URI.parse(original_url).host}
-  end
-
   if @mix_env == :test do
     defp get_final_id(nil, initial_url), do: initial_url
     defp get_final_id("", initial_url), do: initial_url
@@ -371,10 +359,6 @@ defmodule Pleroma.Object.Fetcher do
     with {:ok, %{body: body, status: code, headers: headers, url: final_url}}
          when code in 200..299 <-
            HTTP.Backoff.get(id, headers),
-         remote_host <-
-           URI.parse(final_url).host,
-         {:cross_domain_redirect, false} <-
-           check_crossdomain_redirect(remote_host, id),
          {:has_content_type, {_, content_type}} <-
            {:has_content_type, List.keyfind(headers, "content-type", 0)},
          {:parse_content_type, {:ok, "application", subtype, type_params}} <-
diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs
index d2de0ccc5..84bf0aa05 100644
--- a/test/pleroma/object/fetcher_test.exs
+++ b/test/pleroma/object/fetcher_test.exs
@@ -22,6 +22,7 @@ defmodule Pleroma.Object.FetcherTest do
     |> Jason.decode!()
     |> Map.put("id", id)
     |> Map.put("actor", actor_id)
+    |> Map.put("attributedTo", actor_id)
     |> Jason.encode!()
   end
 
@@ -109,7 +110,7 @@ defmodule Pleroma.Object.FetcherTest do
           body: spoofed_object_with_ids("https://patch.cx/objects/spoof_media_redirect1")
         }
 
-      # Spoof: cross-domain redirect with final domain id
+      # Spoof: cross-domain redirect with final domain id, but original id actor
       %{method: :get, url: "https://patch.cx/objects/spoof_media_redirect2"} ->
         %Tesla.Env{
           status: 200,
@@ -118,6 +119,19 @@ defmodule Pleroma.Object.FetcherTest do
           body: spoofed_object_with_ids("https://media.patch.cx/objects/spoof_media_redirect2")
         }
 
+      # No-Spoof: cross-domain redirect with id and actor from final domain
+      %{method: :get, url: "https://patch.cx/objects/spoof_media_redirect3"} ->
+        %Tesla.Env{
+          status: 200,
+          url: "https://media.patch.cx/objects/spoof_media_redirect3",
+          headers: [{"content-type", "application/activity+json"}],
+          body:
+            spoofed_object_with_ids(
+              "https://media.patch.cx/objects/spoof_media_redirect3",
+              "https://media.patch.cx/users/rin"
+            )
+        }
+
       # No-Spoof: same domain redirect
       %{method: :get, url: "https://patch.cx/objects/spoof_redirect"} ->
         %Tesla.Env{
@@ -264,19 +278,29 @@ defmodule Pleroma.Object.FetcherTest do
     end
 
     test "it does not fetch an object via cross-domain redirects (initial id)" do
-      assert {:error, {:cross_domain_redirect, true}} =
+      assert {:error, {:containment, _}} =
                Fetcher.fetch_and_contain_remote_object_from_id(
                  "https://patch.cx/objects/spoof_media_redirect1"
                )
     end
 
-    test "it does not fetch an object via cross-domain redirects (final id)" do
-      assert {:error, {:cross_domain_redirect, true}} =
+    test "it does not fetch an object via cross-domain redirect if the actor is from the original domain" do
+      assert {:error, {:containment, :error}} =
                Fetcher.fetch_and_contain_remote_object_from_id(
                  "https://patch.cx/objects/spoof_media_redirect2"
                )
     end
 
+    test "it allows cross-domain redirects when id and author are from final domain" do
+      assert {:ok, %{"id" => id, "attributedTo" => author}} =
+               Fetcher.fetch_and_contain_remote_object_from_id(
+                 "https://patch.cx/objects/spoof_media_redirect3"
+               )
+
+      assert URI.parse(id).host == "media.patch.cx"
+      assert URI.parse(author).host == "media.patch.cx"
+    end
+
     test "it accepts same-domain redirects" do
       assert {:ok, %{"id" => id} = _object} =
                Fetcher.fetch_and_contain_remote_object_from_id(