Page MenuHomePhorge
Files F85829836

No OneTemporary
Actions

Authored By
Unknown
Size
9 KB
Referenced Files
None
Subscribers
None

View Options

diff --git a/changelog.d/c2s-hardening.security b/changelog.d/c2s-hardening.security
index b7e2ef4ef..1e756add8 100644
--- a/changelog.d/c2s-hardening.security
+++ b/changelog.d/c2s-hardening.security
@@ -5,3 +5,4 @@ Validate the object of local Updates; the server builds the edit history and edi
Hold new local polls to the poll limits and reject preset vote counts
Reject local replies to and quotes of posts the author cannot see or quote
Local posts and edits only accept own uploads as attachments, within the attachment and description limits, and emoji with shortcode names
+C2S: the server sets publish time, context and other server-owned fields of new objects, scrubs their HTML and counts contentMap toward the character limit
diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex
index dd5f6028f..69d8d9acf 100644
--- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex
+++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex
@@ -7,6 +7,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
alias Pleroma.Activity
alias Pleroma.Delivery
+ alias Pleroma.HTML
alias Pleroma.Object
alias Pleroma.Object.Fetcher
alias Pleroma.User
@@ -532,33 +533,38 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
|> json(err)
end
+ # Fields the server sets for new local objects, like the Builder does for
+ # CommonAPI. Counters and internal fields are stripped as well.
+ @server_set_fields ~w(published updated formerRepresentations replies source url
+ repliesCount quotesCount)
+
defp fix_user_message(%User{ap_id: actor}, %{"type" => "Create", "object" => object} = activity)
when is_map(object) do
- length =
- [object["content"], object["summary"], object["name"]]
- |> Enum.filter(&is_binary(&1))
- |> Enum.join("")
- |> String.length()
+ object =
+ object
+ |> Transmogrifier.strip_internal_fields()
+ |> Map.drop(@server_set_fields)
+ |> normalize_client_content()
+ |> Map.merge(%{
+ "attributedTo" => actor,
+ "actor" => actor,
+ "id" => Utils.generate_object_id(),
+ "published" => Utils.make_date(),
+ "context" => client_object_context(object)
+ })
- limit = Pleroma.Config.get([:instance, :limit])
+ with :ok <- check_character_limit(object) do
+ {:ok, Map.put(activity, "object", object)}
+ end
+ end
- if length < limit do
- object =
- object
- |> Transmogrifier.strip_internal_fields()
- |> Map.put("attributedTo", actor)
- |> Map.put("actor", actor)
- |> Map.put("id", Utils.generate_object_id())
+ # The validator builds the rest of an edit from the stored object.
+ defp fix_user_message(%User{}, %{"type" => "Update", "object" => object} = activity)
+ when is_map(object) do
+ object = normalize_client_content(object)
+ with :ok <- check_character_limit(object) do
{:ok, Map.put(activity, "object", object)}
- else
- {:error,
- dgettext(
- "errors",
- "Character limit (%{limit} characters) exceeded, contains %{length} characters",
- limit: limit,
- length: length
- )}
end
end
@@ -582,6 +588,70 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
{:ok, activity}
end
+ defp normalize_client_content(object) do
+ object
+ |> apply_content_map()
+ |> scrub_client_html()
+ end
+
+ # contentMap would bypass the character limit, so it becomes the content.
+ defp apply_content_map(%{"content" => content} = object)
+ when is_binary(content) and content != "",
+ do: Map.delete(object, "contentMap")
+
+ defp apply_content_map(%{"contentMap" => %{} = content_map} = object) do
+ content = Enum.find_value(content_map, fn {_, value} -> is_binary(value) && value end)
+
+ object
+ |> Map.delete("contentMap")
+ |> Pleroma.Maps.put_if_present("content", content)
+ end
+
+ defp apply_content_map(object), do: Map.delete(object, "contentMap")
+
+ # An Answer's name must match the poll option as it is.
+ defp scrub_client_html(%{"type" => "Answer"} = object), do: object
+
+ defp scrub_client_html(object) do
+ Enum.reduce(~w(content summary name), object, fn field, object ->
+ case object[field] do
+ html when is_binary(html) -> Map.put(object, field, HTML.filter_tags(html))
+ _ -> object
+ end
+ end)
+ end
+
+ defp client_object_context(%{"inReplyTo" => in_reply_to}) when is_binary(in_reply_to) do
+ case Object.normalize(in_reply_to, fetch: false) do
+ %Object{data: %{"context" => context}} when is_binary(context) -> context
+ _ -> Utils.generate_context_id()
+ end
+ end
+
+ defp client_object_context(_object), do: Utils.generate_context_id()
+
+ defp check_character_limit(object) do
+ length =
+ [object["content"], object["summary"], object["name"]]
+ |> Enum.filter(&is_binary(&1))
+ |> Enum.join("")
+ |> String.length()
+
+ limit = Pleroma.Config.get([:instance, :limit])
+
+ if length <= limit do
+ :ok
+ else
+ {:error,
+ dgettext(
+ "errors",
+ "Character limit (%{limit} characters) exceeded, contains %{length} characters",
+ limit: limit,
+ length: length
+ )}
+ end
+ end
+
# We currently lack a Flag ObjectValidator since both CommonAPI and Transmogrifier
# both send it straight to ActivityPub.flag and C2S currently has to go through
# the normal pipeline which requires an ObjectValidator.
@@ -618,6 +688,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
end
end
+ defp validate_visibility(%User{}, _activity), do: {:error, "The activity has no object"}
+
def update_outbox(
%{assigns: %{user: %User{nickname: nickname, ap_id: actor} = user}} = conn,
%{"nickname" => nickname} = params
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 29dd90e0a..7f5e446de 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
@@ -435,4 +435,102 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerC2STest do
post_outbox(conn, insert(:user), note(%{"tag" => [tag]}), 400)
end
end
+
+ describe "server-set fields of new objects" do
+ defp stored(result), do: Object.normalize(result["object"], fetch: false).data
+
+ test "the server sets the publish time and drops edit fields", %{conn: conn} do
+ history = %{
+ "type" => "OrderedCollection",
+ "totalItems" => 1,
+ "orderedItems" => [%{"type" => "Note", "content" => "I never wrote this"}]
+ }
+
+ data =
+ note(%{
+ "published" => "2001-01-01T00:00:00Z",
+ "updated" => "2001-01-02T00:00:00Z",
+ "formerRepresentations" => history
+ })
+
+ stored = stored(post_outbox(conn, insert(:user), data, 201))
+
+ {:ok, published, _} = DateTime.from_iso8601(stored["published"])
+ assert DateTime.diff(DateTime.utc_now(), published) < 60
+ refute Map.has_key?(stored, "updated")
+ refute Map.has_key?(stored, "formerRepresentations")
+ end
+
+ test "a missing publish time is set", %{conn: conn} do
+ stored = stored(post_outbox(conn, insert(:user), note(%{}), 201))
+ assert is_binary(stored["published"])
+ end
+
+ test "drops url, source, replies and counters", %{conn: conn} do
+ data =
+ note(%{
+ "url" => "https://phish.example/",
+ "source" => %{"content" => "different source", "mediaType" => "text/plain"},
+ "replies" => for(i <- 1..5, do: "http://10.0.0.1:#{i}/internal"),
+ "repliesCount" => 999,
+ "quotesCount" => 999
+ })
+
+ stored = stored(post_outbox(conn, insert(:user), data, 201))
+
+ refute stored["url"] == "https://phish.example/"
+ refute Map.has_key?(stored, "source")
+ assert stored["replies"] in [nil, []]
+ assert stored["repliesCount"] in [nil, 0]
+ assert stored["quotesCount"] in [nil, 0]
+ assert all_enqueued(worker: Pleroma.Workers.RemoteFetcherWorker) == []
+ end
+
+ test "the context comes from the replied post, or is new", %{conn: conn} do
+ {:ok, op} = CommonAPI.post(insert(:user), %{status: "my thread"})
+ user = insert(:user)
+
+ injected = stored(post_outbox(conn, user, note(%{"context" => op.data["context"]}), 201))
+ refute injected["context"] == op.data["context"]
+
+ reply_data =
+ note(%{"context" => "https://evil.example/c", "inReplyTo" => op.data["object"]})
+
+ reply = stored(post_outbox(conn, user, reply_data, 201))
+ assert reply["context"] == op.data["context"]
+ end
+
+ test "scrubs HTML like the Mastodon API", %{conn: conn} do
+ html = "<script>alert(1)</script><a href=\"javascript:x\">x</a><b>ok</b>"
+
+ stored =
+ stored(
+ post_outbox(conn, insert(:user), note(%{"content" => html, "summary" => html}), 201)
+ )
+
+ for field <- ["content", "summary"] do
+ refute stored[field] =~ "<script"
+ refute stored[field] =~ "javascript:"
+ end
+ end
+
+ test "contentMap counts toward the character limit", %{conn: conn} do
+ clear_config([:instance, :limit], 100)
+ long = String.duplicate("a", 200)
+
+ data = %{
+ "type" => "Create",
+ "object" => %{"type" => "Note", "contentMap" => %{"en" => long}, "to" => [@public]}
+ }
+
+ post_outbox(conn, insert(:user), data, 400)
+
+ data = put_in(data, ["object", "content"], "short")
+ post_outbox(conn, insert(:user), data, 201)
+ end
+
+ test "a body without an object is a bad request", %{conn: conn} do
+ post_outbox(conn, insert(:user), %{"type" => "Like"}, 400)
+ end
+ end
end

File Metadata

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

Event Timeline