diff --git a/lib/pleroma/object/containment.ex b/lib/pleroma/object/containment.ex index 219bc3892..85b777179 100644 --- a/lib/pleroma/object/containment.ex +++ b/lib/pleroma/object/containment.ex @@ -62,7 +62,7 @@ def contain_origin(id, %{"actor" => _actor} = params) do def contain_origin(id, %{"attributedTo" => actor} = params), do: contain_origin(id, Map.put(params, "actor", actor)) - def contain_origin(_id, _data), do: :error + def contain_origin(_id, _data), do: :ok @doc """ Check whether the object id is from the same host as another id diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 9c0bf7124..16d8194e7 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -143,7 +143,6 @@ def fetch_object_from_id(id, options \\ []) do {_, {:ok, data}} <- {:fetch, fetch_and_contain_remote_object_from_id(id)}, {_, nil} <- {:normalize, Object.normalize(data, fetch: false)}, params <- prepare_activity_params(data), - {_, :ok} <- {:containment, Containment.contain_origin(id, params)}, {_, {:ok, activity}} <- {:transmogrifier, Transmogrifier.handle_incoming(params, options)}, {_, _data, %Object{} = object} <- @@ -156,9 +155,6 @@ def fetch_object_from_id(id, options \\ []) do {:scheme, false} -> {:error, "URI Scheme Invalid"} - {:containment, _} -> - {:error, "Object containment failed."} - {:transmogrifier, {:error, {:reject, e}}} -> {:reject, e} @@ -264,7 +260,8 @@ def fetch_and_contain_remote_object_from_id(id) when is_binary(id) do with {:scheme, true} <- {:scheme, String.starts_with?(id, "http")}, {:ok, body} <- get_object(id), {:ok, data} <- safe_json_decode(body), - :ok <- Containment.contain_origin_from_id(id, data) do + {_, :ok} <- {:containment, Containment.contain_origin_from_id(id, data)}, + {_, :ok} <- {:containment, Containment.contain_origin(id, data)} do unless Instances.reachable?(id) do Instances.set_reachable(id) end @@ -274,6 +271,9 @@ def fetch_and_contain_remote_object_from_id(id) when is_binary(id) do {:scheme, _} -> {:error, "Unsupported URI scheme"} + {:containment, _} -> + {:error, "Object containment failed."} + {:error, e} -> {:error, e} diff --git a/test/pleroma/object/containment_test.exs b/test/pleroma/object/containment_test.exs index fb2fb7d49..ad1660069 100644 --- a/test/pleroma/object/containment_test.exs +++ b/test/pleroma/object/containment_test.exs @@ -17,16 +17,58 @@ defmodule Pleroma.Object.ContainmentTest do end describe "general origin containment" do - test "works for completely actorless posts" do - assert :error == - Containment.contain_origin("https://glaceon.social/users/monorail", %{ + test "handles completly actorless objects gracefully" do + assert :ok == + Containment.contain_origin("https://glaceon.social/statuses/123", %{ "deleted" => "2019-10-30T05:48:50.249606Z", "formerType" => "Note", - "id" => "https://glaceon.social/users/monorail/statuses/103049757364029187", + "id" => "https://glaceon.social/statuses/123", "type" => "Tombstone" }) end + test "errors for spoofed actors" do + assert :error == + Containment.contain_origin("https://glaceon.social/statuses/123", %{ + "actor" => "https://otp.akkoma.dev/users/you", + "id" => "https://glaceon.social/statuses/123", + "type" => "Note" + }) + end + + test "errors for spoofed attributedTo" do + assert :error == + Containment.contain_origin("https://glaceon.social/statuses/123", %{ + "attributedTo" => "https://otp.akkoma.dev/users/you", + "id" => "https://glaceon.social/statuses/123", + "type" => "Note" + }) + end + + test "accepts valid actors" do + assert :ok == + Containment.contain_origin("https://glaceon.social/statuses/123", %{ + "actor" => "https://glaceon.social/users/monorail", + "attributedTo" => "https://glaceon.social/users/monorail", + "id" => "https://glaceon.social/statuses/123", + "type" => "Note" + }) + + assert :ok == + Containment.contain_origin("https://glaceon.social/statuses/123", %{ + "actor" => "https://glaceon.social/users/monorail", + "id" => "https://glaceon.social/statuses/123", + "type" => "Note" + }) + + assert :ok == + Containment.contain_origin("https://glaceon.social/statuses/123", %{ + "attributedTo" => "https://glaceon.social/users/monorail", + "id" => "https://glaceon.social/statuses/123", + "type" => "Note" + }) + end + test "contain_origin_from_id() catches obvious spoofing attempts" do data = %{ "id" => "http://example.com/~alyssa/activities/1234.json" diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 7f6f9c031..b289e869d 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -84,6 +84,19 @@ defp spoofed_object_with_ids( body: spoofed_object_with_ids("https://patch.cx/objects/spoof_redirect") } + # Spoof: Actor from another domain + %{method: :get, url: "https://patch.cx/objects/spoof_foreign_actor"} -> + %Tesla.Env{ + status: 200, + url: "https://patch.cx/objects/spoof_foreign_actor", + headers: [{"content-type", "application/activity+json"}], + body: + spoofed_object_with_ids( + "https://patch.cx/objects/spoof_foreign_actor", + "https://not.patch.cx/users/rin" + ) + } + env -> apply(HttpRequestMock, :request, [env]) end) @@ -217,6 +230,13 @@ test "it accepts same-domain redirects" do assert id == "https://patch.cx/objects/spoof_redirect" end + + test "it does not fetch a spoofed object with a foreign actor" do + assert {:error, "Object containment failed."} = + Fetcher.fetch_and_contain_remote_object_from_id( + "https://patch.cx/objects/spoof_foreign_actor" + ) + end end describe "fetching an object" do