From c4cf4d7f0bb1d1082f74aea8fb54ed50160ab29e Mon Sep 17 00:00:00 2001 From: Oneric Date: Wed, 13 Mar 2024 20:12:17 -0100 Subject: [PATCH] Reject cross-domain redirects when fetching AP objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Such redirects on AP queries seem most likely to be a spoofing attempt. If the object is legit, the id should match the final domain anyway and users can directly use the canonical URL. The lack of such a check (and use of the initially queried domain’s authority instead of the final domain) was enabling the current exploit to even affect instances which already migrated away from a same-domain upload/proxy setup in the past, but retained a redirect to not break old attachments. (In theory this redirect could, with some effort, have been limited to only old files, but common guides employed a catch-all redirect, which allows even future uploads to be reachable via an initial query to the main domain) Same-domain redirects are valid and also used by ourselves, e.g. for redirecting /notice/XXX to /objects/YYY. --- lib/pleroma/object/fetcher.ex | 25 +++++++++++++- test/pleroma/object/fetcher_test.exs | 50 ++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index c3aaf7a03..9c0bf7124 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -26,6 +26,8 @@ defmodule Pleroma.Object.Fetcher do function use the former and perform some additional tasks """ + @mix_env Mix.env() + defp touch_changeset(changeset) do updated_at = NaiveDateTime.utc_now() @@ -283,6 +285,22 @@ defmodule Pleroma.Object.Fetcher do def fetch_and_contain_remote_object_from_id(_id), do: {:error, "id must be a string"} + 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 + @doc "Do NOT use; only public for use in tests" def get_object(id) do date = Pleroma.Signature.signed_date() @@ -292,8 +310,13 @@ defmodule Pleroma.Object.Fetcher do |> maybe_date_fetch(date) |> sign_fetch(id, date) - with {:ok, %{body: body, status: code, headers: headers}} when code in 200..299 <- + with {:ok, %{body: body, status: code, headers: headers, url: final_url}} + when code in 200..299 <- HTTP.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 b2da0a757..7f6f9c031 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -57,6 +57,33 @@ defmodule Pleroma.Object.FetcherTest do body: spoofed_object_with_ids("https://patch.cx/objects/spoof_content_type") } + # Spoof: cross-domain redirect with original domain id + %{method: :get, url: "https://patch.cx/objects/spoof_media_redirect1"} -> + %Tesla.Env{ + status: 200, + url: "https://media.patch.cx/objects/spoof", + headers: [{"content-type", "application/activity+json"}], + body: spoofed_object_with_ids("https://patch.cx/objects/spoof_media_redirect1") + } + + # Spoof: cross-domain redirect with final domain id + %{method: :get, url: "https://patch.cx/objects/spoof_media_redirect2"} -> + %Tesla.Env{ + status: 200, + url: "https://media.patch.cx/objects/spoof_media_redirect2", + headers: [{"content-type", "application/activity+json"}], + body: spoofed_object_with_ids("https://media.patch.cx/objects/spoof_media_redirect2") + } + + # No-Spoof: same domain redirect + %{method: :get, url: "https://patch.cx/objects/spoof_redirect"} -> + %Tesla.Env{ + status: 200, + url: "https://patch.cx/objects/spoof", + headers: [{"content-type", "application/activity+json"}], + body: spoofed_object_with_ids("https://patch.cx/objects/spoof_redirect") + } + env -> apply(HttpRequestMock, :request, [env]) end) @@ -167,6 +194,29 @@ defmodule Pleroma.Object.FetcherTest do "https://patch.cx/objects/spoof_content_type.json" ) end + + test "it does not fetch an object via cross-domain redirects (initial id)" do + assert {:error, {:cross_domain_redirect, true}} = + 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}} = + Fetcher.fetch_and_contain_remote_object_from_id( + "https://patch.cx/objects/spoof_media_redirect2" + ) + end + + test "it accepts same-domain redirects" do + assert {:ok, %{"id" => id} = _object} = + Fetcher.fetch_and_contain_remote_object_from_id( + "https://patch.cx/objects/spoof_redirect" + ) + + assert id == "https://patch.cx/objects/spoof_redirect" + end end describe "fetching an object" do