From f1337d81593315c90e79840a21b1372e9f794d57 Mon Sep 17 00:00:00 2001 From: Lain Soykaf Date: Tue, 27 Oct 2026 21:46:14 +0400 Subject: [PATCH] fix(fetcher): keep id and author fixed when refetching objects refetch_object (used for poll updates) accepted whatever the remote served. A hostile server could swap in another object or move the post to another author. The refetch now goes to the stored id without redirects, and the returned id and actor must equal the stored ones. Based on Akkoma 9061d148b and fee57eb37. --- changelog.d/refetch-containment.security | 1 + lib/pleroma/object/fetcher.ex | 12 +++++- test/pleroma/object/fetcher_test.exs | 54 ++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 changelog.d/refetch-containment.security diff --git a/changelog.d/refetch-containment.security b/changelog.d/refetch-containment.security new file mode 100644 index 000000000..bfd118ab1 --- /dev/null +++ b/changelog.d/refetch-containment.security @@ -0,0 +1 @@ +Do not let a refetched object (such as a poll) change its id or author diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 8a6f5561f..706ee317c 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -11,6 +11,7 @@ defmodule Pleroma.Object.Fetcher do alias Pleroma.Web.ActivityPub.InternalFetchActor alias Pleroma.Web.ActivityPub.MRF alias Pleroma.Web.ActivityPub.ObjectValidator + alias Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.Federator @@ -46,9 +47,14 @@ defmodule Pleroma.Object.Fetcher do end end - def refetch_object(%Object{data: %{"id" => id}} = object) do + # The stored id is canonical, so the refetch must not redirect, and the + # remote must not swap in another object or author. + # Based on Akkoma 9061d148b and fee57eb37. + def refetch_object(%Object{data: %{"id" => id} = data} = object) do with {:local, false} <- {:local, Object.local?(object)}, - {:ok, new_data} <- fetch_and_contain_remote_object_from_id(id), + {:ok, new_data} <- fetch_and_contain(id, true), + {:id, true} <- {:id, new_data["id"] == id}, + {:actor, true} <- {:actor, actor_of(new_data) == actor_of(data)}, {:ok, object} <- reinject_object(object, new_data) do {:ok, object} else @@ -57,6 +63,8 @@ defmodule Pleroma.Object.Fetcher do end end + defp actor_of(data), do: CommonFixes.fix_actor(data)["actor"] + @typep fetcher_errors :: :error | :reject | :allowed_depth | :fetch | :containment | :transmogrifier diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 411f69bea..c9c4890cd 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -465,6 +465,60 @@ defmodule Pleroma.Object.FetcherTest do %{object1: local_object1, object2: local_object2} end + test "it rejects a refetched object whose actor changed", %{object1: object1} do + insert(:user, ap_id: "https://mastodon.social/users/someone_else") + {:ok, o} = Object.create(object1) + + mock(fn %{method: :get, url: "https://mastodon.social/1"} -> + %Tesla.Env{ + status: 200, + headers: [{"content-type", "application/activity+json"}], + body: + object1 + |> Map.merge(%{ + "actor" => "https://mastodon.social/users/someone_else", + "attributedTo" => "https://mastodon.social/users/someone_else", + "content" => "hijacked" + }) + |> Jason.encode!() + } + end) + + assert {:error, _} = Fetcher.refetch_object(o) + assert Object.get_by_ap_id("https://mastodon.social/1").data["content"] == "test 1" + end + + test "it rejects a refetched object whose id changed", %{object1: object1} do + {:ok, o} = Object.create(object1) + + mock(fn %{method: :get, url: "https://mastodon.social/1"} -> + %Tesla.Env{ + status: 200, + url: "https://mastodon.social/2", + headers: [{"content-type", "application/activity+json"}], + body: object1 |> Map.put("id", "https://mastodon.social/2") |> Jason.encode!() + } + end) + + assert {:error, _} = Fetcher.refetch_object(o) + end + + test "it rejects a refetched object whose id differs only by a trailing slash", %{ + object1: object1 + } do + {:ok, o} = Object.create(object1) + + mock(fn %{method: :get, url: "https://mastodon.social/1"} -> + %Tesla.Env{ + status: 200, + headers: [{"content-type", "application/activity+json"}], + body: object1 |> Map.put("id", "https://mastodon.social/1/") |> Jason.encode!() + } + end) + + assert {:error, _} = Fetcher.refetch_object(o) + end + test "it keeps formerRepresentations if remote does not have this attr", %{object1: object1} do full_object1 = object1