Page MenuHomePhorge
Files F85829830

No OneTemporary
Actions

Authored By
Unknown
Size
11 KB
Referenced Files
None
Subscribers
None

View Options

diff --git a/changelog.d/c2s-hardening.security b/changelog.d/c2s-hardening.security
index 5974de72c..9bb46d986 100644
--- a/changelog.d/c2s-hardening.security
+++ b/changelog.d/c2s-hardening.security
@@ -1,2 +1,3 @@
Reject EmojiReact activities whose name is not a single emoji shortcode
Enforce poll voting rules (visible, open poll, not the author, one vote per option) for all votes on local polls and all local votes
+Only allow pinning own posts, and only public or unlisted ones for local users; the featured collection skips non-public posts
diff --git a/lib/pleroma/web/activity_pub/object_validator.ex b/lib/pleroma/web/activity_pub/object_validator.ex
index b57d9d562..49a1b9238 100644
--- a/lib/pleroma/web/activity_pub/object_validator.ex
+++ b/lib/pleroma/web/activity_pub/object_validator.ex
@@ -284,7 +284,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do
def validate(%{"type" => type} = object, meta) when type in ~w(Add Remove) do
with {:ok, object} <-
object
- |> AddRemoveValidator.cast_and_validate()
+ |> AddRemoveValidator.cast_and_validate(meta)
|> Ecto.Changeset.apply_action(:insert) do
object = stringify_keys(object)
{:ok, object, meta}
diff --git a/lib/pleroma/web/activity_pub/object_validators/add_remove_validator.ex b/lib/pleroma/web/activity_pub/object_validators/add_remove_validator.ex
index db3259550..64d30a5b7 100644
--- a/lib/pleroma/web/activity_pub/object_validators/add_remove_validator.ex
+++ b/lib/pleroma/web/activity_pub/object_validators/add_remove_validator.ex
@@ -10,7 +10,9 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.AddRemoveValidator do
require Pleroma.Constants
+ alias Pleroma.Object
alias Pleroma.User
+ alias Pleroma.Web.ActivityPub.Visibility
@primary_key false
@@ -26,7 +28,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.AddRemoveValidator do
end
end
- def cast_and_validate(data) do
+ def cast_and_validate(data, meta \\ []) do
{:ok, actor} = User.get_or_fetch_by_ap_id(data["actor"])
{:ok, actor} = maybe_refetch_user(actor)
@@ -34,7 +36,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.AddRemoveValidator do
data
|> maybe_fix_data_for_mastodon(actor)
|> cast_data()
- |> validate_data(actor)
+ |> validate_data(actor, meta)
end
defp maybe_fix_data_for_mastodon(data, actor) do
@@ -49,13 +51,38 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.AddRemoveValidator do
cast(%__MODULE__{}, data, __schema__(:fields))
end
- defp validate_data(changeset, actor) do
+ defp validate_data(changeset, actor, meta) do
changeset
|> validate_required([:id, :target, :object, :actor, :type, :to, :cc])
|> validate_inclusion(:type, ~w(Add Remove))
|> validate_actor_presence()
|> validate_collection_belongs_to_actor(actor)
|> validate_object_presence()
+ |> validate_pinned_object(actor, meta)
+ end
+
+ # Remove stays possible for any object, so old pins can always be undone.
+ defp validate_pinned_object(%{changes: %{type: "Add"}} = changeset, actor, meta) do
+ validate_change(changeset, :object, fn :object, object_id ->
+ object = Object.get_cached_by_ap_id(object_id)
+
+ cond do
+ is_nil(object) -> [object: "is not an object"]
+ object.data["actor"] != actor.ap_id -> [object: "doesn't belong to the actor"]
+ meta[:local] -> validate_local_pin(object)
+ true -> []
+ end
+ end)
+ end
+
+ defp validate_pinned_object(changeset, _actor, _meta), do: changeset
+
+ defp validate_local_pin(%Object{data: %{"type" => type}} = object) do
+ cond do
+ type not in ~w(Note Article Question) -> [object: "can't be pinned"]
+ !Visibility.public?(object) -> [object: "is not public"]
+ true -> []
+ end
end
defp validate_collection_belongs_to_actor(changeset, actor) do
diff --git a/lib/pleroma/web/activity_pub/views/user_view.ex b/lib/pleroma/web/activity_pub/views/user_view.ex
index c273043e1..9651c7188 100644
--- a/lib/pleroma/web/activity_pub/views/user_view.ex
+++ b/lib/pleroma/web/activity_pub/views/user_view.ex
@@ -13,6 +13,7 @@ defmodule Pleroma.Web.ActivityPub.UserView do
alias Pleroma.Web.ActivityPub.ObjectView
alias Pleroma.Web.ActivityPub.Transmogrifier
alias Pleroma.Web.ActivityPub.Utils
+ alias Pleroma.Web.ActivityPub.Visibility
alias Pleroma.Web.Endpoint
alias Pleroma.Web.Router.Helpers
@@ -257,9 +258,9 @@ defmodule Pleroma.Web.ActivityPub.UserView do
objects =
pinned_objects
|> Enum.sort_by(fn {_, pinned_at} -> pinned_at end, &>=/2)
- |> Enum.map(fn {id, _} ->
- ObjectView.render("object.json", %{object: Object.get_cached_by_ap_id(id)})
- end)
+ |> Enum.map(fn {id, _} -> Object.get_cached_by_ap_id(id) end)
+ |> Enum.filter(&(&1 && Visibility.public?(&1)))
+ |> Enum.map(&ObjectView.render("object.json", %{object: &1}))
%{
"id" => featured_address,
diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex
index ca7ea94b1..fbe58bd25 100644
--- a/lib/pleroma/web/common_api.ex
+++ b/lib/pleroma/web/common_api.ex
@@ -588,9 +588,6 @@ defmodule Pleroma.Web.CommonAPI do
def pin(id, %User{} = user) do
with %Activity{} = activity <- create_activity_by_id(id),
true <- activity_visible_to_actor(activity, user),
- true <- activity_belongs_to_actor(activity, user.ap_id),
- true <- object_type_is_allowed_for_pin(activity.object),
- true <- activity_is_public(activity),
{:ok, pin_data, _} <- Builder.pin(user, activity.object),
{:ok, _pin, _} <-
Pipeline.common_pipeline(pin_data,
@@ -600,10 +597,20 @@ defmodule Pleroma.Web.CommonAPI do
{:ok, activity}
else
{:error, {:side_effects, error}} -> error
+ {:error, {:validate, {:error, changeset}}} -> {:error, pin_error(changeset)}
error -> error
end
end
+ defp pin_error(%Ecto.Changeset{errors: errors}) do
+ case errors[:object] do
+ {"doesn't belong to the actor", _} -> :ownership_error
+ {"can't be pinned", _} -> :not_allowed
+ {"is not public", _} -> :non_public_error
+ _ -> :not_allowed
+ end
+ end
+
defp create_activity_by_id(id) do
with nil <- Activity.create_by_id_with_object(id) do
{:error, :not_found}
@@ -621,18 +628,6 @@ defmodule Pleroma.Web.CommonAPI do
end
end
- defp object_type_is_allowed_for_pin(%{data: %{"type" => type}}) do
- with false <- type in ["Note", "Article", "Question"] do
- {:error, :not_allowed}
- end
- end
-
- defp activity_is_public(activity) do
- with false <- Visibility.public?(activity) do
- {:error, :non_public_error}
- end
- end
-
@spec unpin(String.t(), User.t()) :: {:ok, Activity.t()} | Pipeline.errors()
def unpin(id, user) do
# Order of visibility/belonging matters for MastoAPI responses.
diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_c2s_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_c2s_test.exs
index 09b9d2cff..c62160738 100644
--- a/test/pleroma/web/activity_pub/activity_pub_controller_c2s_test.exs
+++ b/test/pleroma/web/activity_pub/activity_pub_controller_c2s_test.exs
@@ -137,4 +137,52 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerC2STest do
assert vote_counts(poll.data["object"]) == %{"a" => 0, "b" => 0}
end
end
+
+ describe "Add (pins)" do
+ defp pin(user, object_id),
+ do: %{"type" => "Add", "target" => user.featured_address, "object" => object_id}
+
+ defp featured(user) do
+ build_conn()
+ |> put_req_header("accept", "application/activity+json")
+ |> get("/users/#{user.nickname}/collections/featured")
+ |> json_response(200)
+ |> Map.get("orderedItems")
+ end
+
+ test "rejects pins of other users' posts", %{conn: conn} do
+ bob = insert(:user)
+ alice = insert(:user)
+ {:ok, alice, bob} = Pleroma.User.follow(alice, bob)
+
+ {:ok, private} = CommonAPI.post(bob, %{status: "followers", visibility: "private"})
+ {:ok, dm} = CommonAPI.post(bob, %{status: "@#{alice.nickname} dm", visibility: "direct"})
+ {:ok, public} = CommonAPI.post(bob, %{status: "public"})
+
+ for post <- [private, dm, public] do
+ post_outbox(conn, alice, pin(alice, post.data["object"]), 400)
+ end
+
+ assert featured(alice) == []
+ end
+
+ test "pins only public or unlisted own posts", %{conn: conn} do
+ alice = insert(:user)
+ {:ok, private} = CommonAPI.post(alice, %{status: "followers", visibility: "private"})
+ {:ok, unlisted} = CommonAPI.post(alice, %{status: "unlisted", visibility: "unlisted"})
+
+ post_outbox(conn, alice, pin(alice, private.data["object"]), 400)
+ post_outbox(conn, alice, pin(alice, unlisted.data["object"]), 201)
+
+ assert [%{"content" => "unlisted"}] = featured(alice)
+ end
+
+ test "the featured collection hides non-public posts that are already pinned" do
+ alice = insert(:user)
+ {:ok, private} = CommonAPI.post(alice, %{status: "followers", visibility: "private"})
+ {:ok, alice} = Pleroma.User.add_pinned_object_id(alice, private.data["object"])
+
+ assert featured(alice) == []
+ end
+ end
end
diff --git a/test/pleroma/web/activity_pub/object_validators/add_remove_validation_test.exs b/test/pleroma/web/activity_pub/object_validators/add_remove_validation_test.exs
new file mode 100644
index 000000000..1fc93f608
--- /dev/null
+++ b/test/pleroma/web/activity_pub/object_validators/add_remove_validation_test.exs
@@ -0,0 +1,50 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.ActivityPub.ObjectValidators.AddRemoveValidationTest do
+ use Pleroma.DataCase, async: true
+
+ alias Pleroma.Web.ActivityPub.ObjectValidator
+ alias Pleroma.Web.CommonAPI
+
+ import Pleroma.Factory
+
+ require Pleroma.Constants
+
+ defp collection_activity(type, user, object_id) do
+ %{
+ "id" => "#{user.ap_id}/activities/#{Ecto.UUID.generate()}",
+ "type" => type,
+ "actor" => user.ap_id,
+ "object" => object_id,
+ "target" => user.featured_address,
+ "to" => [Pleroma.Constants.as_public()],
+ "cc" => [user.follower_address]
+ }
+ end
+
+ setup do
+ {:ok, post} = CommonAPI.post(insert(:user), %{status: "not yours"})
+ %{object_id: post.data["object"]}
+ end
+
+ test "a remote Add of another actor's object is rejected", %{object_id: object_id} do
+ remote =
+ insert(:user,
+ local: false,
+ ap_id: "https://remote.example/users/pinner",
+ featured_address: "https://remote.example/users/pinner/collections/featured"
+ )
+
+ add = collection_activity("Add", remote, object_id)
+ assert {:error, %Ecto.Changeset{}} = ObjectValidator.validate(add, local: false)
+ end
+
+ test "a Remove of another actor's object is accepted, so bad pins can go away",
+ %{object_id: object_id} do
+ user = insert(:user)
+ remove = collection_activity("Remove", user, object_id)
+ assert {:ok, _, _} = ObjectValidator.validate(remove, local: true)
+ end
+end

File Metadata

Mime Type
text/x-diff
Expires
Sun, Nov 1, 12:50 AM (16 h, 1 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1801995
Default Alt Text
(11 KB)

Event Timeline