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

@@ -0,0 +1 @@
+Do not let a refetched object (such as a poll) change its id or author

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
 

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