commit: adb5cb96d38d24d0756fd42e6ae84c4c95c6f758
parent 577b7cb0618eeb9617978d631c08ec72ca8cb19d
Author: Lain Soykaf <lain@lain.com>
Date: Tue, 11 Mar 2025 15:50:17 +0400
Object.Fetcher: Don't do cross-site redirects.
Diffstat:
2 files changed, 152 insertions(+), 10 deletions(-)
diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex
@@ -19,6 +19,8 @@ defmodule Pleroma.Object.Fetcher do
require Logger
require Pleroma.Constants
+ @mix_env Mix.env()
+
@spec reinject_object(struct(), map()) :: {:ok, Object.t()} | {:error, any()}
defp reinject_object(%Object{data: %{}} = object, new_data) do
Logger.debug("Reinjecting object #{new_data["id"]}")
@@ -172,6 +174,19 @@ 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)
+
+ # Handle the common case in tests where responses don't include URLs
+ 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
+
defp get_object(id) do
date = Pleroma.Signature.signed_date()
@@ -181,19 +196,29 @@ defmodule Pleroma.Object.Fetcher do
|> sign_fetch(id, date)
case HTTP.get(id, headers) do
+ {:ok, %{body: body, status: code, headers: headers, url: final_url}}
+ when code in 200..299 ->
+ remote_host = if final_url, do: URI.parse(final_url).host, else: nil
+
+ with {:cross_domain_redirect, false} <- check_crossdomain_redirect(remote_host, id),
+ {_, content_type} <- List.keyfind(headers, "content-type", 0),
+ {:ok, _media_type} <- verify_content_type(content_type) do
+ {:ok, body}
+ else
+ {:cross_domain_redirect, true} ->
+ {:error, {:cross_domain_redirect, true}}
+
+ error ->
+ error
+ end
+
+ # Handle the case where URL is not in the response (older HTTP library versions)
{:ok, %{body: body, status: code, headers: headers}} when code in 200..299 ->
case List.keyfind(headers, "content-type", 0) do
{_, content_type} ->
- case Plug.Conn.Utils.media_type(content_type) do
- {:ok, "application", "activity+json", _} ->
- {:ok, body}
-
- {:ok, "application", "ld+json",
- %{"profile" => "https://www.w3.org/ns/activitystreams"}} ->
- {:ok, body}
-
- _ ->
- {:error, {:content_type, content_type}}
+ case verify_content_type(content_type) do
+ {:ok, _} -> {:ok, body}
+ error -> error
end
_ ->
@@ -216,4 +241,17 @@ defmodule Pleroma.Object.Fetcher do
defp safe_json_decode(nil), do: {:ok, nil}
defp safe_json_decode(json), do: Jason.decode(json)
+
+ defp verify_content_type(content_type) do
+ case Plug.Conn.Utils.media_type(content_type) do
+ {:ok, "application", "activity+json", _} ->
+ {:ok, :activity_json}
+
+ {:ok, "application", "ld+json", %{"profile" => "https://www.w3.org/ns/activitystreams"}} ->
+ {:ok, :ld_json}
+
+ _ ->
+ {:error, {:content_type, content_type}}
+ end
+ end
end
diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs
@@ -534,6 +534,110 @@ defmodule Pleroma.Object.FetcherTest do
end
end
+ describe "cross-domain redirect handling" do
+ setup do
+ mock(fn
+ # Cross-domain redirect with original domain in id
+ %{method: :get, url: "https://original.test/objects/123"} ->
+ %Tesla.Env{
+ status: 200,
+ url: "https://media.test/objects/123",
+ headers: [{"content-type", "application/activity+json"}],
+ body:
+ Jason.encode!(%{
+ "id" => "https://original.test/objects/123",
+ "type" => "Note",
+ "content" => "This is redirected content",
+ "actor" => "https://original.test/users/actor",
+ "attributedTo" => "https://original.test/users/actor"
+ })
+ }
+
+ # Cross-domain redirect with final domain in id
+ %{method: :get, url: "https://original.test/objects/final-domain-id"} ->
+ %Tesla.Env{
+ status: 200,
+ url: "https://media.test/objects/final-domain-id",
+ headers: [{"content-type", "application/activity+json"}],
+ body:
+ Jason.encode!(%{
+ "id" => "https://media.test/objects/final-domain-id",
+ "type" => "Note",
+ "content" => "This has final domain in id",
+ "actor" => "https://original.test/users/actor",
+ "attributedTo" => "https://original.test/users/actor"
+ })
+ }
+
+ # No redirect - same domain
+ %{method: :get, url: "https://original.test/objects/same-domain-redirect"} ->
+ %Tesla.Env{
+ status: 200,
+ url: "https://original.test/objects/different-path",
+ headers: [{"content-type", "application/activity+json"}],
+ body:
+ Jason.encode!(%{
+ "id" => "https://original.test/objects/same-domain-redirect",
+ "type" => "Note",
+ "content" => "This has a same-domain redirect",
+ "actor" => "https://original.test/users/actor",
+ "attributedTo" => "https://original.test/users/actor"
+ })
+ }
+
+ # Test case with missing url field in response (common in tests)
+ %{method: :get, url: "https://original.test/objects/missing-url"} ->
+ %Tesla.Env{
+ status: 200,
+ # No url field
+ headers: [{"content-type", "application/activity+json"}],
+ body:
+ Jason.encode!(%{
+ "id" => "https://original.test/objects/missing-url",
+ "type" => "Note",
+ "content" => "This has no URL field in response",
+ "actor" => "https://original.test/users/actor",
+ "attributedTo" => "https://original.test/users/actor"
+ })
+ }
+ end)
+
+ :ok
+ end
+
+ test "it rejects objects from cross-domain redirects with original domain in id" do
+ assert {:error, {:cross_domain_redirect, true}} =
+ Fetcher.fetch_and_contain_remote_object_from_id(
+ "https://original.test/objects/123"
+ )
+ end
+
+ test "it rejects objects from cross-domain redirects with final domain in id" do
+ assert {:error, {:cross_domain_redirect, true}} =
+ Fetcher.fetch_and_contain_remote_object_from_id(
+ "https://original.test/objects/final-domain-id"
+ )
+ end
+
+ test "it accepts objects with same-domain redirects" do
+ assert {:ok, data} =
+ Fetcher.fetch_and_contain_remote_object_from_id(
+ "https://original.test/objects/same-domain-redirect"
+ )
+
+ assert data["content"] == "This has a same-domain redirect"
+ end
+
+ test "it handles responses without URL field (common in tests)" do
+ assert {:ok, data} =
+ Fetcher.fetch_and_contain_remote_object_from_id(
+ "https://original.test/objects/missing-url"
+ )
+
+ assert data["content"] == "This has no URL field in response"
+ end
+ end
+
describe "fetch with history" do
setup do
object2 = %{
