Merge branch 'hotfix/leaking-lists' into 'develop'

Mastodon API: Fix lists leaking private posts

See merge request pleroma/pleroma!1222

CHANGELOG.md

@@ -119,6 +119,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

lib/pleroma/web/activity_pub/activity_pub.ex

@@ -649,20 +649,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
@@ -885,9 +871,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

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)

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

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