From a9eaa558853460b811d134b49fb00b017b772e94 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Fri, 31 May 2019 14:17:05 +0300 Subject: [PATCH 1/2] Fix fix_explicit_addressing moving follower collection to cc and add tests for it --- .../web/activity_pub/transmogrifier.ex | 5 ++- test/web/activity_pub/transmogrifier_test.exs | 40 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 5edd8ccc7..d8fa2728d 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -93,7 +93,10 @@ def fix_explicit_addressing(object) do object |> Utils.determine_explicit_mentions() - explicit_mentions = explicit_mentions ++ ["https://www.w3.org/ns/activitystreams#Public"] + follower_collection = User.get_cached_by_ap_id(Containment.get_actor(object)).follower_address + + explicit_mentions = + explicit_mentions ++ ["https://www.w3.org/ns/activitystreams#Public", follower_collection] object |> fix_explicit_addressing(explicit_mentions) diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index c24b50f8c..ee71de8d0 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -1209,4 +1209,44 @@ test "successfully reserializes a message with AS2 objects in IR" do {:ok, _} = Transmogrifier.prepare_outgoing(activity.data) end end + + describe "fix_explicit_addressing" do + test "moves non-explicitly mentioned actors to cc" do + user = insert(:user) + + explicitly_mentioned_actors = [ + "https://pleroma.gold/users/user1", + "https://pleroma.gold/user2" + ] + + object = %{ + "actor" => user.ap_id, + "to" => explicitly_mentioned_actors ++ ["https://social.beepboop.ga/users/dirb"], + "cc" => [], + "tag" => + Enum.map(explicitly_mentioned_actors, fn href -> + %{"type" => "Mention", "href" => href} + end) + } + + fixed_object = Transmogrifier.fix_explicit_addressing(object) + assert Enum.all?(explicitly_mentioned_actors, &(&1 in fixed_object["to"])) + refute "https://social.beepboop.ga/users/dirb" in fixed_object["to"] + assert "https://social.beepboop.ga/users/dirb" in fixed_object["cc"] + end + + test "does not move actor's follower collection to cc" do + user = insert(:user) + + object = %{ + "actor" => user.ap_id, + "to" => [user.follower_address], + "cc" => [] + } + + fixed_object = Transmogrifier.fix_explicit_addressing(object) + assert user.follower_address in fixed_object["to"] + refute user.follower_address in fixed_object["cc"] + end + end end From d9c0650ff9afd66c15d960b727dc2e6ed37477a3 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Fri, 31 May 2019 15:25:17 +0300 Subject: [PATCH 2/2] Mastodon API: Fix lists leaking private posts Our previous list visibility resolver grabbed posts if either follower collection of the user in a list who is followed is in `to` or if follower collection of the user in a list was in `cc`. This not only missed unlisted posts but also lead to leaking private posts when `fix_explicit_addressing` mistakingly started putting follower collections to `cc` (also fixed in this MR). Reported by @kurisu@iscute.moe via a DM --- CHANGELOG.md | 4 +++ lib/pleroma/web/activity_pub/activity_pub.ex | 27 ++++++++---------- test/web/activity_pub/activity_pub_test.exs | 29 ++++++++++++++++++++ 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f689160e9..716cf2a46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -118,6 +118,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Removed - Configuration: `config :pleroma, :fe` in favor of the more flexible `config :pleroma, :frontend_configurations` +## [0.9.99999] - 2019-05-31 +### Security +- Mastodon API: Fix lists leaking private posts + ## [0.9.9999] - 2019-04-05 ### Security - Mastodon API: Fix content warnings skipping HTML sanitization diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index aa0229db7..498b1a342 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -653,20 +653,6 @@ defp restrict_tag(query, %{"tag" => tag}) when is_binary(tag) do defp restrict_tag(query, _), do: query - defp restrict_to_cc(query, recipients_to, recipients_cc) do - from( - activity in query, - where: - fragment( - "(?->'to' \\?| ?) or (?->'cc' \\?| ?)", - activity.data, - ^recipients_to, - activity.data, - ^recipients_cc - ) - ) - end - defp restrict_recipients(query, [], _user), do: query defp restrict_recipients(query, recipients, nil) do @@ -889,9 +875,18 @@ def fetch_activities(recipients, opts \\ %{}) do |> Enum.reverse() end - def fetch_activities_bounded(recipients_to, recipients_cc, opts \\ %{}) do + def fetch_activities_bounded_query(query, recipients, recipients_with_public) do + from(activity in query, + where: + fragment("? && ?", activity.recipients, ^recipients) or + (fragment("? && ?", activity.recipients, ^recipients_with_public) and + "https://www.w3.org/ns/activitystreams#Public" in activity.recipients) + ) + end + + def fetch_activities_bounded(recipients, recipients_with_public, opts \\ %{}) do fetch_activities_query([], opts) - |> restrict_to_cc(recipients_to, recipients_cc) + |> fetch_activities_bounded_query(recipients, recipients_with_public) |> Pagination.fetch_paginated(opts) |> Enum.reverse() end diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index f743f380b..76586ee4a 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -1186,4 +1186,33 @@ test "it can create a Flag activity" do def data_uri do File.read!("test/fixtures/avatar_data_uri") end + + describe "fetch_activities_bounded" do + test "fetches private posts for followed users" do + user = insert(:user) + + {:ok, activity} = + CommonAPI.post(user, %{ + "status" => "thought I looked cute might delete later :3", + "visibility" => "private" + }) + + [result] = ActivityPub.fetch_activities_bounded([user.follower_address], []) + assert result.id == activity.id + end + + test "fetches only public posts for other users" do + user = insert(:user) + {:ok, activity} = CommonAPI.post(user, %{"status" => "#cofe", "visibility" => "public"}) + + {:ok, _private_activity} = + CommonAPI.post(user, %{ + "status" => "why is tenshi eating a corndog so cute?", + "visibility" => "private" + }) + + [result] = ActivityPub.fetch_activities_bounded([], [user.follower_address]) + assert result.id == activity.id + end + end end