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(