From 0b2c051a04b3eeb7292f2b847c98fcbafbb20ed2 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 1 Sep 2018 23:20:02 +0000 Subject: [PATCH 1/3] activitypub: fix possibility of spoofing by containing remote objects to the same domain as their actor --- lib/pleroma/web/activity_pub/activity_pub.ex | 1 + lib/pleroma/web/activity_pub/transmogrifier.ex | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index e6c2dc9cf..81c11dd76 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -747,6 +747,7 @@ def fetch_object_from_id(id) do "actor" => data["attributedTo"], "object" => data }, + :ok <- Transmogrifier.contain_origin(id, params), {:ok, activity} <- Transmogrifier.handle_incoming(params) do {:ok, Object.normalize(activity.data["object"])} else diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 1367bc7e3..b75422fc6 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -30,6 +30,20 @@ def get_actor(%{"actor" => actor}) when is_map(actor) do actor["id"] end + @doc """ + Checks that an imported AP object's actor matches the domain it came from. + """ + def contain_origin(id, %{"actor" => actor}) do + id_uri = URI.parse(id) + actor_uri = URI.parse(actor) + + if id_uri.host == actor_uri.host do + :ok + else + :error + end + end + @doc """ Modifies an incoming AP object (mastodon format) to our internal format. """ From 303af9ba4c4b4b079f0d1ef474dda7afc13e30e0 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 1 Sep 2018 23:33:10 +0000 Subject: [PATCH 2/3] tests: add regression tests --- .../https__info.pleroma.site_activity.json | 14 +++++++++++++ test/support/httpoison_mock.ex | 8 +++++++ test/web/activity_pub/transmogrifier_test.exs | 21 +++++++++++++++++++ 3 files changed, 43 insertions(+) create mode 100644 test/fixtures/httpoison_mock/https__info.pleroma.site_activity.json diff --git a/test/fixtures/httpoison_mock/https__info.pleroma.site_activity.json b/test/fixtures/httpoison_mock/https__info.pleroma.site_activity.json new file mode 100644 index 000000000..eab0341fe --- /dev/null +++ b/test/fixtures/httpoison_mock/https__info.pleroma.site_activity.json @@ -0,0 +1,14 @@ +{ + "@context": "https://www.w3.org/ns/activitystreams", + "actor": "https://mastodon.example.org/users/admin", + "attachment": [], + "attributedTo": "https://mastodon.example.org/users/admin", + "content": "

this post was not actually written by Haelwenn

", + "id": "https://info.pleroma.site/activity.json", + "published": "2018-09-01T22:15:00Z", + "tag": [], + "to": [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "type": "Note" +} diff --git a/test/support/httpoison_mock.ex b/test/support/httpoison_mock.ex index 4ee2feb95..7057f30fb 100644 --- a/test/support/httpoison_mock.ex +++ b/test/support/httpoison_mock.ex @@ -3,6 +3,14 @@ defmodule HTTPoisonMock do def get(url, body \\ [], headers \\ []) + def get("https://info.pleroma.site/activity.json", _, _) do + {:ok, + %Response{ + status_code: 200, + body: File.read!("test/fixtures/httpoison_mock/https__info.pleroma.site_activity.json") + }} + end + def get("https://puckipedia.com/", [Accept: "application/activity+json"], _) do {:ok, %Response{ diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index e2926d495..afa25bb60 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -798,4 +798,25 @@ test "it fixes the actor URL property to be a proper URI" do assert rewritten["url"] == "http://example.com" end end + + describe "actor origin containment" do + test "it rejects objects with a bogus origin" do + {:error, _} = ActivityPub.fetch_object_from_id("https://info.pleroma.site/activity.json") + end + + test "it rejects activities which reference objects with bogus origins" do + user = insert(:user, %{local: false}) + + data = %{ + "@context" => "https://www.w3.org/ns/activitystreams", + "id" => user.ap_id <> "/activities/1234", + "actor" => user.ap_id, + "to" => ["https://www.w3.org/ns/activitystreams#Public"], + "object" => "https://info.pleroma.site/activity.json", + "type" => "Announce" + } + + :error = Transmogrifier.handle_incoming(data) + end + end end From 03e92977cb95ccc81b92c927049a3e4421917cd2 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 1 Sep 2018 23:44:19 +0000 Subject: [PATCH 3/3] transmogrifier: fix peertube/plume actor handling --- lib/pleroma/web/activity_pub/transmogrifier.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index b75422fc6..4a3a82195 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -33,9 +33,9 @@ def get_actor(%{"actor" => actor}) when is_map(actor) do @doc """ Checks that an imported AP object's actor matches the domain it came from. """ - def contain_origin(id, %{"actor" => actor}) do + def contain_origin(id, %{"actor" => actor} = params) do id_uri = URI.parse(id) - actor_uri = URI.parse(actor) + actor_uri = URI.parse(get_actor(params)) if id_uri.host == actor_uri.host do :ok