From 675639225a905f5b0b2650cd3f20a4758fc3f868 Mon Sep 17 00:00:00 2001 From: HJ <30-hj@users.noreply.git.pleroma.social> Date: Fri, 28 Apr 2023 11:13:42 +0000 Subject: [PATCH 01/27] allow https: so that flash works across instances without need for media proxy --- lib/pleroma/web/plugs/http_security_plug.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/plugs/http_security_plug.ex b/lib/pleroma/web/plugs/http_security_plug.ex index 34895c8d5..045384e08 100644 --- a/lib/pleroma/web/plugs/http_security_plug.ex +++ b/lib/pleroma/web/plugs/http_security_plug.ex @@ -104,7 +104,7 @@ defmodule Pleroma.Web.Plugs.HTTPSecurityPlug do {[img_src, " https:"], [media_src, " https:"]} end - connect_src = ["connect-src 'self' blob: ", static_url, ?\s, websocket_url] + connect_src = ["connect-src 'self' blob: https: ", static_url, ?\s, websocket_url] connect_src = if Config.get(:env) == :dev do From cd20d15bb8d2f97f8dd0850993041f15865cdda9 Mon Sep 17 00:00:00 2001 From: HJ <30-hj@users.noreply.git.pleroma.social> Date: Fri, 28 Apr 2023 11:19:14 +0000 Subject: [PATCH 02/27] changelog --- changelog.d/3879.fix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3879.fix diff --git a/changelog.d/3879.fix b/changelog.d/3879.fix new file mode 100644 index 000000000..7c58cc3c2 --- /dev/null +++ b/changelog.d/3879.fix @@ -0,0 +1 @@ +fix not being able to fetch flash file from remote instance \ No newline at end of file From c0d11da2d8edc57ef88163c06a19aad3e28d14db Mon Sep 17 00:00:00 2001 From: Henry Jameson Date: Sun, 7 May 2023 15:16:30 +0300 Subject: [PATCH 03/27] conditionally set csp depnding on media-proxy state --- lib/pleroma/web/plugs/http_security_plug.ex | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/web/plugs/http_security_plug.ex b/lib/pleroma/web/plugs/http_security_plug.ex index 045384e08..df46cfa0c 100644 --- a/lib/pleroma/web/plugs/http_security_plug.ex +++ b/lib/pleroma/web/plugs/http_security_plug.ex @@ -93,18 +93,26 @@ defmodule Pleroma.Web.Plugs.HTTPSecurityPlug do img_src = "img-src 'self' data: blob:" media_src = "media-src 'self'" + connect_src = ["connect-src 'self' blob:", static_url, ?\s, websocket_url] # Strict multimedia CSP enforcement only when MediaProxy is enabled - {img_src, media_src} = + {img_src, media_src, connect_src} = if Config.get([:media_proxy, :enabled]) && !Config.get([:media_proxy, :proxy_opts, :redirect_on_failure]) do sources = build_csp_multimedia_source_list() - {[img_src, sources], [media_src, sources]} + { + [img_src, sources], + [media_src, sources], + [connect_src, sources] + } else - {[img_src, " https:"], [media_src, " https:"]} + { + [img_src, " https:"], + [media_src, " https:"], + [connect_src, " https:"] + } end - connect_src = ["connect-src 'self' blob: https: ", static_url, ?\s, websocket_url] connect_src = if Config.get(:env) == :dev do From f8ef4924ecab5ba6851eee82845624bc15f868de Mon Sep 17 00:00:00 2001 From: Henry Jameson Date: Sun, 7 May 2023 15:24:09 +0300 Subject: [PATCH 04/27] fix whitespace --- lib/pleroma/web/plugs/http_security_plug.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/plugs/http_security_plug.ex b/lib/pleroma/web/plugs/http_security_plug.ex index df46cfa0c..a3166bc96 100644 --- a/lib/pleroma/web/plugs/http_security_plug.ex +++ b/lib/pleroma/web/plugs/http_security_plug.ex @@ -93,7 +93,7 @@ defmodule Pleroma.Web.Plugs.HTTPSecurityPlug do img_src = "img-src 'self' data: blob:" media_src = "media-src 'self'" - connect_src = ["connect-src 'self' blob:", static_url, ?\s, websocket_url] + connect_src = ["connect-src 'self' blob: ", static_url, ?\s, websocket_url] # Strict multimedia CSP enforcement only when MediaProxy is enabled {img_src, media_src, connect_src} = From f50fd9278fd36e6bd3ae36bb7f5033d9fd8a84ac Mon Sep 17 00:00:00 2001 From: Henry Jameson Date: Sun, 7 May 2023 15:29:19 +0300 Subject: [PATCH 05/27] reduce redundant reduntancy reduction --- lib/pleroma/web/plugs/http_security_plug.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/plugs/http_security_plug.ex b/lib/pleroma/web/plugs/http_security_plug.ex index a3166bc96..b189d5bfd 100644 --- a/lib/pleroma/web/plugs/http_security_plug.ex +++ b/lib/pleroma/web/plugs/http_security_plug.ex @@ -93,7 +93,7 @@ defmodule Pleroma.Web.Plugs.HTTPSecurityPlug do img_src = "img-src 'self' data: blob:" media_src = "media-src 'self'" - connect_src = ["connect-src 'self' blob: ", static_url, ?\s, websocket_url] + connect_src = "connect-src 'self' blob:" # Strict multimedia CSP enforcement only when MediaProxy is enabled {img_src, media_src, connect_src} = @@ -103,7 +103,7 @@ defmodule Pleroma.Web.Plugs.HTTPSecurityPlug do { [img_src, sources], [media_src, sources], - [connect_src, sources] + [connect_src, sources, ?\s, websocket_url] } else { From 2a07411b0cb14ea26966659605d95074b02a8538 Mon Sep 17 00:00:00 2001 From: Henry Jameson Date: Sun, 7 May 2023 15:34:17 +0300 Subject: [PATCH 06/27] keep the websocket url for all modes --- lib/pleroma/web/plugs/http_security_plug.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/plugs/http_security_plug.ex b/lib/pleroma/web/plugs/http_security_plug.ex index b189d5bfd..b3dc8a3a6 100644 --- a/lib/pleroma/web/plugs/http_security_plug.ex +++ b/lib/pleroma/web/plugs/http_security_plug.ex @@ -93,7 +93,7 @@ defmodule Pleroma.Web.Plugs.HTTPSecurityPlug do img_src = "img-src 'self' data: blob:" media_src = "media-src 'self'" - connect_src = "connect-src 'self' blob:" + connect_src = ["connect-src 'self' blob: ", ?\s, websocket_url] # Strict multimedia CSP enforcement only when MediaProxy is enabled {img_src, media_src, connect_src} = @@ -103,7 +103,7 @@ defmodule Pleroma.Web.Plugs.HTTPSecurityPlug do { [img_src, sources], [media_src, sources], - [connect_src, sources, ?\s, websocket_url] + [connect_src, sources] } else { From ea4225a646b355150fb8e5e8c77d7fdc58b5e7ef Mon Sep 17 00:00:00 2001 From: tusooa Date: Tue, 18 Jul 2023 18:39:59 -0400 Subject: [PATCH 07/27] Restrict attachments to only uploaded files only --- changelog.d/attachment-type-check.fix | 1 + lib/pleroma/constants.ex | 2 ++ lib/pleroma/web/common_api/utils.ex | 7 ++++++- test/pleroma/web/common_api/utils_test.exs | 11 ++++++++--- 4 files changed, 17 insertions(+), 4 deletions(-) create mode 100644 changelog.d/attachment-type-check.fix diff --git a/changelog.d/attachment-type-check.fix b/changelog.d/attachment-type-check.fix new file mode 100644 index 000000000..9e14b75f1 --- /dev/null +++ b/changelog.d/attachment-type-check.fix @@ -0,0 +1 @@ +Restrict attachments to only uploaded files only diff --git a/lib/pleroma/constants.ex b/lib/pleroma/constants.ex index 7b4fd03b6..6befc6897 100644 --- a/lib/pleroma/constants.ex +++ b/lib/pleroma/constants.ex @@ -81,4 +81,6 @@ defmodule Pleroma.Constants do const(mime_regex, do: ~r/^[^[:cntrl:] ()<>@,;:\\"\/\[\]?=]+\/[^[:cntrl:] ()<>@,;:\\"\/\[\]?=]+(; .*)?$/ ) + + const(upload_object_types, do: ["Document", "Image"]) end diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex index a93c97e1e..b9fe0224c 100644 --- a/lib/pleroma/web/common_api/utils.ex +++ b/lib/pleroma/web/common_api/utils.ex @@ -59,7 +59,12 @@ defmodule Pleroma.Web.CommonAPI.Utils do end defp get_attachment(media_id) do - Repo.get(Object, media_id) + with %Object{data: data} = object <- Repo.get(Object, media_id), + %{"type" => type} when type in Pleroma.Constants.upload_object_types() <- data do + object + else + _ -> nil + end end @spec get_to_and_cc(ActivityDraft.t()) :: {list(String.t()), list(String.t())} diff --git a/test/pleroma/web/common_api/utils_test.exs b/test/pleroma/web/common_api/utils_test.exs index d309c6ded..ca5b92683 100644 --- a/test/pleroma/web/common_api/utils_test.exs +++ b/test/pleroma/web/common_api/utils_test.exs @@ -592,7 +592,7 @@ defmodule Pleroma.Web.CommonAPI.UtilsTest do end test "returns list attachments with desc" do - object = insert(:note) + object = insert(:attachment) desc = Jason.encode!(%{object.id => "test-desc"}) assert Utils.attachments_from_ids_descs(["#{object.id}", "34"], desc) == [ @@ -603,7 +603,7 @@ defmodule Pleroma.Web.CommonAPI.UtilsTest do describe "attachments_from_ids/1" do test "returns attachments with descs" do - object = insert(:note) + object = insert(:attachment) desc = Jason.encode!(%{object.id => "test-desc"}) assert Utils.attachments_from_ids(%{ @@ -615,13 +615,18 @@ defmodule Pleroma.Web.CommonAPI.UtilsTest do end test "returns attachments without descs" do - object = insert(:note) + object = insert(:attachment) assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}) == [object.data] end test "returns [] when not pass media_ids" do assert Utils.attachments_from_ids(%{}) == [] end + + test "checks that the object is of upload type" do + object = insert(:note) + assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}) == [] + end end describe "maybe_add_list_data/3" do From dc4de79d43ba941d8aa16f7c14887faae9f65e9f Mon Sep 17 00:00:00 2001 From: faried nawaz Date: Wed, 7 Dec 2022 22:37:50 +0500 Subject: [PATCH 08/27] status context: perform visibility check on activities around a status issue #2927 --- lib/pleroma/web/activity_pub/activity_pub.ex | 23 ++ .../controllers/status_controller_test.exs | 267 ++++++++++++++++++ 2 files changed, 290 insertions(+) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index c93288b79..f1a12d22d 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -455,6 +455,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do |> maybe_preload_objects(opts) |> maybe_preload_bookmarks(opts) |> maybe_set_thread_muted_field(opts) + |> restrict_unauthenticated(opts[:user]) |> restrict_blocked(opts) |> restrict_blockers_visibility(opts) |> restrict_recipients(recipients, opts[:user]) @@ -1215,6 +1216,28 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do defp restrict_filtered(query, _), do: query + defp restrict_unauthenticated(query, nil) do + local = Config.restrict_unauthenticated_access?(:activities, :local) + remote = Config.restrict_unauthenticated_access?(:activities, :remote) + + cond do + local and remote -> + # is there a better way to handle this? + from(activity in query, where: 1 == 0) + + local -> + from(activity in query, where: activity.local == false) + + remote -> + from(activity in query, where: activity.local == true) + + true -> + query + end + end + + defp restrict_unauthenticated(query, _), do: query + defp exclude_poll_votes(query, %{include_poll_votes: true}), do: query defp exclude_poll_votes(query, _) do diff --git a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs index 4f434cb69..76c289ee7 100644 --- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs @@ -771,6 +771,49 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do {:ok, local: local, remote: remote} end + defp local_and_remote_context_activities do + local_user_1 = insert(:user) + local_user_2 = insert(:user) + remote_user = insert(:user, local: false) + + {:ok, %{id: id1, data: %{"context" => context}}} = + CommonAPI.post(local_user_1, %{status: "post"}) + + {:ok, %{id: id2} = post} = + CommonAPI.post(local_user_2, %{status: "local reply", in_reply_to_status_id: id1}) + + params = %{ + "@context" => "https://www.w3.org/ns/activitystreams", + "actor" => remote_user.ap_id, + "type" => "Create", + "context" => context, + "id" => "#{remote_user.ap_id}/activities/1", + "inReplyTo" => post.data["id"], + "object" => %{ + "type" => "Note", + "content" => "remote reply", + "context" => context, + "id" => "#{remote_user.ap_id}/objects/1", + "attributedTo" => remote_user.ap_id, + "to" => [ + local_user_1.ap_id, + local_user_2.ap_id, + "https://www.w3.org/ns/activitystreams#Public" + ] + }, + "to" => [ + local_user_1.ap_id, + local_user_2.ap_id, + "https://www.w3.org/ns/activitystreams#Public" + ] + } + + {:ok, job} = Pleroma.Web.Federator.incoming_ap_doc(params) + {:ok, remote_activity} = ObanHelpers.perform(job) + + %{locals: [id1, id2], remote: remote_activity.id, context: context} + end + describe "status with restrict unauthenticated activities for local and remote" do setup do: local_and_remote_activities() @@ -957,6 +1000,230 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do end end + describe "getting status contexts restricted unauthenticated for local and remote" do + setup do: local_and_remote_context_activities() + + setup do: clear_config([:restrict_unauthenticated, :activities, :local], true) + + setup do: clear_config([:restrict_unauthenticated, :activities, :remote], true) + + test "if user is unauthenticated", %{conn: conn, locals: [post_id, _]} do + res_conn = get(conn, "/api/v1/statuses/#{post_id}/context") + + assert json_response_and_validate_schema(res_conn, 200) == %{ + "ancestors" => [], + "descendants" => [] + } + end + + test "if user is unauthenticated reply", %{conn: conn, locals: [_, reply_id]} do + res_conn = get(conn, "/api/v1/statuses/#{reply_id}/context") + + assert json_response_and_validate_schema(res_conn, 200) == %{ + "ancestors" => [], + "descendants" => [] + } + end + + test "if user is authenticated", %{locals: [post_id, reply_id], remote: remote_reply_id} do + %{conn: conn} = oauth_access(["read"]) + res_conn = get(conn, "/api/v1/statuses/#{post_id}/context") + + %{"ancestors" => [], "descendants" => descendants} = + json_response_and_validate_schema(res_conn, 200) + + descendant_ids = + descendants + |> Enum.map(& &1["id"]) + + assert reply_id in descendant_ids + assert remote_reply_id in descendant_ids + end + + test "if user is authenticated reply", %{locals: [post_id, reply_id], remote: remote_reply_id} do + %{conn: conn} = oauth_access(["read"]) + res_conn = get(conn, "/api/v1/statuses/#{reply_id}/context") + + %{"ancestors" => ancestors, "descendants" => descendants} = + json_response_and_validate_schema(res_conn, 200) + + ancestor_ids = + ancestors + |> Enum.map(& &1["id"]) + + descendant_ids = + descendants + |> Enum.map(& &1["id"]) + + assert post_id in ancestor_ids + assert remote_reply_id in descendant_ids + end + end + + describe "getting status contexts restricted unauthenticated for local" do + setup do: local_and_remote_context_activities() + + setup do: clear_config([:restrict_unauthenticated, :activities, :local], true) + + setup do: clear_config([:restrict_unauthenticated, :activities, :remote], false) + + test "if user is unauthenticated", %{ + conn: conn, + locals: [post_id, reply_id], + remote: remote_reply_id + } do + res_conn = get(conn, "/api/v1/statuses/#{post_id}/context") + + %{"ancestors" => [], "descendants" => descendants} = + json_response_and_validate_schema(res_conn, 200) + + descendant_ids = + descendants + |> Enum.map(& &1["id"]) + + assert reply_id not in descendant_ids + assert remote_reply_id in descendant_ids + end + + test "if user is unauthenticated reply", %{ + conn: conn, + locals: [post_id, reply_id], + remote: remote_reply_id + } do + res_conn = get(conn, "/api/v1/statuses/#{reply_id}/context") + + %{"ancestors" => ancestors, "descendants" => descendants} = + json_response_and_validate_schema(res_conn, 200) + + ancestor_ids = + ancestors + |> Enum.map(& &1["id"]) + + descendant_ids = + descendants + |> Enum.map(& &1["id"]) + + assert post_id not in ancestor_ids + assert remote_reply_id in descendant_ids + end + + test "if user is authenticated", %{locals: [post_id, reply_id], remote: remote_reply_id} do + %{conn: conn} = oauth_access(["read"]) + res_conn = get(conn, "/api/v1/statuses/#{post_id}/context") + + %{"ancestors" => [], "descendants" => descendants} = + json_response_and_validate_schema(res_conn, 200) + + descendant_ids = + descendants + |> Enum.map(& &1["id"]) + + assert reply_id in descendant_ids + assert remote_reply_id in descendant_ids + end + + test "if user is authenticated reply", %{locals: [post_id, reply_id], remote: remote_reply_id} do + %{conn: conn} = oauth_access(["read"]) + res_conn = get(conn, "/api/v1/statuses/#{reply_id}/context") + + %{"ancestors" => ancestors, "descendants" => descendants} = + json_response_and_validate_schema(res_conn, 200) + + ancestor_ids = + ancestors + |> Enum.map(& &1["id"]) + + descendant_ids = + descendants + |> Enum.map(& &1["id"]) + + assert post_id in ancestor_ids + assert remote_reply_id in descendant_ids + end + end + + describe "getting status contexts restricted unauthenticated for remote" do + setup do: local_and_remote_context_activities() + + setup do: clear_config([:restrict_unauthenticated, :activities, :local], false) + + setup do: clear_config([:restrict_unauthenticated, :activities, :remote], true) + + test "if user is unauthenticated", %{ + conn: conn, + locals: [post_id, reply_id], + remote: remote_reply_id + } do + res_conn = get(conn, "/api/v1/statuses/#{post_id}/context") + + %{"ancestors" => [], "descendants" => descendants} = + json_response_and_validate_schema(res_conn, 200) + + descendant_ids = + descendants + |> Enum.map(& &1["id"]) + + assert reply_id in descendant_ids + assert remote_reply_id not in descendant_ids + end + + test "if user is unauthenticated reply", %{ + conn: conn, + locals: [post_id, reply_id], + remote: remote_reply_id + } do + res_conn = get(conn, "/api/v1/statuses/#{reply_id}/context") + + %{"ancestors" => ancestors, "descendants" => descendants} = + json_response_and_validate_schema(res_conn, 200) + + ancestor_ids = + ancestors + |> Enum.map(& &1["id"]) + + descendant_ids = + descendants + |> Enum.map(& &1["id"]) + + assert post_id in ancestor_ids + assert remote_reply_id not in descendant_ids + end + + test "if user is authenticated", %{locals: [post_id, reply_id], remote: remote_reply_id} do + %{conn: conn} = oauth_access(["read"]) + res_conn = get(conn, "/api/v1/statuses/#{post_id}/context") + + %{"ancestors" => [], "descendants" => descendants} = + json_response_and_validate_schema(res_conn, 200) + + reply_ids = + descendants + |> Enum.map(& &1["id"]) + + assert reply_id in reply_ids + assert remote_reply_id in reply_ids + end + + test "if user is authenticated reply", %{locals: [post_id, reply_id], remote: remote_reply_id} do + %{conn: conn} = oauth_access(["read"]) + res_conn = get(conn, "/api/v1/statuses/#{reply_id}/context") + + %{"ancestors" => ancestors, "descendants" => descendants} = + json_response_and_validate_schema(res_conn, 200) + + ancestor_ids = + ancestors + |> Enum.map(& &1["id"]) + + descendant_ids = + descendants + |> Enum.map(& &1["id"]) + + assert post_id in ancestor_ids + assert remote_reply_id in descendant_ids + end + end + describe "deleting a status" do test "when you created it" do %{user: author, conn: conn} = oauth_access(["write:statuses"]) From e5e76ec44559af62eb56043add0489c61e6c1ee5 Mon Sep 17 00:00:00 2001 From: Faried Nawaz Date: Fri, 10 Feb 2023 01:32:32 +0500 Subject: [PATCH 09/27] cleaner ecto query to handle restrict_unauthenticated for activities This fix is for this case: config :pleroma, :restrict_unauthenticated, activities: %{local: true, remote: true} --- lib/pleroma/web/activity_pub/activity_pub.ex | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index f1a12d22d..3979d418e 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1222,8 +1222,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do cond do local and remote -> - # is there a better way to handle this? - from(activity in query, where: 1 == 0) + from(activity in query, where: false) local -> from(activity in query, where: activity.local == false) From 11ce81d4af6b428fabb9d4c6f0098d786a21487b Mon Sep 17 00:00:00 2001 From: faried nawaz Date: Fri, 28 Jul 2023 18:49:05 +0500 Subject: [PATCH 10/27] add changelog entry --- changelog.d/3801.fix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3801.fix diff --git a/changelog.d/3801.fix b/changelog.d/3801.fix new file mode 100644 index 000000000..8c2ec0199 --- /dev/null +++ b/changelog.d/3801.fix @@ -0,0 +1 @@ +Filter context activities using Visibility.visible_for_user? From 2c795094535537a8607cc0d3b7f076a609636f40 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 3 Aug 2023 13:08:37 -0400 Subject: [PATCH 11/27] Resolve information disclosure vulnerability through emoji pack archive download endpoint The pack name has been sanitized so an attacker cannot upload a media file called pack.json with their own handcrafted list of emoji files as arbitrary files on the filesystem and then call the emoji pack archive download endpoint with a pack name crafted to the location of the media file they uploaded which tricks Pleroma into generating a zip file of the target files the attacker wants to download. The attack only works if the Pleroma instance does not have the AnonymizeFilename upload filter enabled, which is currently the default. Reported by: graf@poast.org --- changelog.d/emoji-pack-sanitization.security | 1 + lib/pleroma/emoji/pack.ex | 1 + test/pleroma/emoji/pack_test.exs | 4 ++++ 3 files changed, 6 insertions(+) create mode 100644 changelog.d/emoji-pack-sanitization.security diff --git a/changelog.d/emoji-pack-sanitization.security b/changelog.d/emoji-pack-sanitization.security new file mode 100644 index 000000000..f3218abd4 --- /dev/null +++ b/changelog.d/emoji-pack-sanitization.security @@ -0,0 +1 @@ +Emoji pack loader sanitizes pack names diff --git a/lib/pleroma/emoji/pack.ex b/lib/pleroma/emoji/pack.ex index a361ea200..6e58f8898 100644 --- a/lib/pleroma/emoji/pack.ex +++ b/lib/pleroma/emoji/pack.ex @@ -285,6 +285,7 @@ defmodule Pleroma.Emoji.Pack do @spec load_pack(String.t()) :: {:ok, t()} | {:error, :file.posix()} def load_pack(name) do + name = Path.basename(name) pack_file = Path.join([emoji_path(), name, "pack.json"]) with {:ok, _} <- File.stat(pack_file), diff --git a/test/pleroma/emoji/pack_test.exs b/test/pleroma/emoji/pack_test.exs index 18b99da75..00001abfc 100644 --- a/test/pleroma/emoji/pack_test.exs +++ b/test/pleroma/emoji/pack_test.exs @@ -90,4 +90,8 @@ defmodule Pleroma.Emoji.PackTest do assert updated_pack.files_count == 1 end + + test "load_pack/1 ignores path traversal in a forged pack name", %{pack: pack} do + assert {:ok, ^pack} = Pack.load_pack("../../../../../dump_pack") + end end From 8cc8100120abdbf26cfe4cdac2c0a012d7919e05 Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Thu, 22 Jun 2023 00:46:52 +0200 Subject: [PATCH 12/27] Config: Restrict permissions of OTP config file --- lib/pleroma/config/release_runtime_provider.ex | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/pleroma/config/release_runtime_provider.ex b/lib/pleroma/config/release_runtime_provider.ex index 91e5f1a54..9ec0f975e 100644 --- a/lib/pleroma/config/release_runtime_provider.ex +++ b/lib/pleroma/config/release_runtime_provider.ex @@ -20,6 +20,20 @@ defmodule Pleroma.Config.ReleaseRuntimeProvider do with_runtime_config = if File.exists?(config_path) do + # + %File.Stat{mode: mode} = File.lstat!(config_path) + + if Bitwise.band(mode, 0o007) > 0 do + raise "Configuration at #{config_path} has world-permissions, execute the following: chmod o= #{config_path}" + end + + if Bitwise.band(mode, 0o020) > 0 do + raise "Configuration at #{config_path} has group-wise write permissions, execute the following: chmod g-w #{config_path}" + end + + # Note: Elixir doesn't provides a getuid(2) + # so cannot forbid group-read only when config is owned by us + runtime_config = Config.Reader.read!(config_path) with_defaults From 69caedc591bd61029f897f37ef7ecddd470cf935 Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Thu, 22 Jun 2023 00:58:05 +0200 Subject: [PATCH 13/27] instance gen: Reduce permissions of pleroma directories and config files --- lib/mix/tasks/pleroma/instance.ex | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/mix/tasks/pleroma/instance.ex b/lib/mix/tasks/pleroma/instance.ex index 5c93f19ff..5d8b254a2 100644 --- a/lib/mix/tasks/pleroma/instance.ex +++ b/lib/mix/tasks/pleroma/instance.ex @@ -266,12 +266,20 @@ defmodule Mix.Tasks.Pleroma.Instance do config_dir = Path.dirname(config_path) psql_dir = Path.dirname(psql_path) + # Note: Distros requiring group read (0o750) on those directories should + # pre-create the directories. [config_dir, psql_dir, static_dir, uploads_dir] |> Enum.reject(&File.exists?/1) - |> Enum.map(&File.mkdir_p!/1) + |> Enum.each(fn dir -> + File.mkdir_p!(dir) + File.chmod!(dir, 0o700) + end) shell_info("Writing config to #{config_path}.") + # Sadly no fchmod(2) equivalent in Elixir… + File.touch!(config_path) + File.chmod!(config_path, 0o640) File.write(config_path, result_config) shell_info("Writing the postgres script to #{psql_path}.") File.write(psql_path, result_psql) @@ -290,8 +298,7 @@ defmodule Mix.Tasks.Pleroma.Instance do else shell_error( "The task would have overwritten the following files:\n" <> - (Enum.map(will_overwrite, &"- #{&1}\n") |> Enum.join("")) <> - "Rerun with `--force` to overwrite them." + Enum.map_join(will_overwrite, &"- #{&1}\n") <> "Rerun with `--force` to overwrite them." ) end end From 9f0ad901ed4f8f0ad3e1d896fd41d25b93a97d76 Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Thu, 22 Jun 2023 01:00:25 +0200 Subject: [PATCH 14/27] changelog: Entry for config permissions restrictions Closes: https://git.pleroma.social/pleroma/pleroma/-/issues/3135 --- changelog.d/otp_perms.security | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/otp_perms.security diff --git a/changelog.d/otp_perms.security b/changelog.d/otp_perms.security new file mode 100644 index 000000000..a3da1c677 --- /dev/null +++ b/changelog.d/otp_perms.security @@ -0,0 +1 @@ +- Reduced permissions of config files and directories, distros requiring greater permissions like group-read need to pre-create the directories \ No newline at end of file From 65ef8f19c5e21baadfa7daabdbd37fb9a1fbf863 Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Fri, 4 Aug 2023 07:16:50 +0200 Subject: [PATCH 15/27] release_runtime_provider_test: chmod config for hardened permissions Git doesn't manages file permissions precisely enough for us. --- test/pleroma/config/release_runtime_provider_test.exs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/pleroma/config/release_runtime_provider_test.exs b/test/pleroma/config/release_runtime_provider_test.exs index e6d10b13e..8d2a93d6c 100644 --- a/test/pleroma/config/release_runtime_provider_test.exs +++ b/test/pleroma/config/release_runtime_provider_test.exs @@ -17,6 +17,8 @@ defmodule Pleroma.Config.ReleaseRuntimeProviderTest do end test "merged runtime config" do + assert :ok == File.chmod!("test/fixtures/config/temp.secret.exs", 0o640) + merged = ReleaseRuntimeProvider.load([], config_path: "test/fixtures/config/temp.secret.exs") @@ -25,6 +27,8 @@ defmodule Pleroma.Config.ReleaseRuntimeProviderTest do end test "merged exported config" do + assert :ok == File.chmod!("test/fixtures/config/temp.exported_from_db.secret.exs", 0o640) + ExUnit.CaptureIO.capture_io(fn -> merged = ReleaseRuntimeProvider.load([], @@ -37,6 +41,9 @@ defmodule Pleroma.Config.ReleaseRuntimeProviderTest do end test "runtime config is merged with exported config" do + assert :ok == File.chmod!("test/fixtures/config/temp.secret.exs", 0o640) + assert :ok == File.chmod!("test/fixtures/config/temp.exported_from_db.secret.exs", 0o640) + merged = ReleaseRuntimeProvider.load([], config_path: "test/fixtures/config/temp.secret.exs", From 6a0fd77c48946b4b2100585b4f32d125680f5f82 Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Fri, 4 Aug 2023 05:13:28 +0200 Subject: [PATCH 16/27] Release 2.5.53 --- CHANGELOG.md | 6 ++++++ mix.exs | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42a1bbb8f..87e9c5298 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Removed - BREAKING: Support for passwords generated with `crypt(3)` (Gnu Social migration artifact) +## 2.5.3 + +### Security +- Emoji pack loader sanitizes pack names +- Reduced permissions of config files and directories, distros requiring greater permissions like group-read need to pre-create the directories + ## 2.5.2 ### Security diff --git a/mix.exs b/mix.exs index fc4e87ee3..115e7113f 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Pleroma.Mixfile do def project do [ app: :pleroma, - version: version("2.5.52"), + version: version("2.5.53"), elixir: "~> 1.11", elixirc_paths: elixirc_paths(Mix.env()), compilers: [:phoenix] ++ Mix.compilers(), From 0e321698d21766772aa2b54b518dcd76a6abce8c Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Fri, 4 Aug 2023 17:09:50 +0200 Subject: [PATCH 17/27] gentoo_otp_en.md: Indicate which install method it covers --- changelog.d/gentoo_otp_intro.skip | 0 docs/installation/gentoo_otp_en.md | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 changelog.d/gentoo_otp_intro.skip diff --git a/changelog.d/gentoo_otp_intro.skip b/changelog.d/gentoo_otp_intro.skip new file mode 100644 index 000000000..e69de29bb diff --git a/docs/installation/gentoo_otp_en.md b/docs/installation/gentoo_otp_en.md index 76968eb38..4fafc0c17 100644 --- a/docs/installation/gentoo_otp_en.md +++ b/docs/installation/gentoo_otp_en.md @@ -2,7 +2,7 @@ {! backend/installation/otp_vs_from_source.include !} -A [manual installation guide for gentoo](./gentoo_en.md) is also available. +This guide covers installation via Gentoo provided packaging. A [manual installation guide for gentoo](./gentoo_en.md) is also available. ## Installation From ca0859b90f0f3cb9bb369d38d29868de59796c2c Mon Sep 17 00:00:00 2001 From: Mae Date: Fri, 4 Aug 2023 22:24:17 +0100 Subject: [PATCH 18/27] Prevent XML parser from loading external entities --- lib/pleroma/web/xml.ex | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/xml.ex b/lib/pleroma/web/xml.ex index b699446b0..380a80ab8 100644 --- a/lib/pleroma/web/xml.ex +++ b/lib/pleroma/web/xml.ex @@ -29,7 +29,10 @@ defmodule Pleroma.Web.XML do {doc, _rest} = text |> :binary.bin_to_list() - |> :xmerl_scan.string(quiet: true) + |> :xmerl_scan.string( + quiet: true, + fetch_fun: fn _, _ -> raise "Resolving external entities not supported" end + ) {:ok, doc} rescue From 307692cee8cdd0dbe3e6cf40c1192fcf43910610 Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Fri, 4 Aug 2023 22:24:32 +0100 Subject: [PATCH 19/27] Add unit test for external entity loading --- test/fixtures/xml_external_entities.xml | 3 +++ test/pleroma/web/web_finger_test.exs | 23 +++++++++++++++++++++++ test/pleroma/web/xml_test.exs | 10 ++++++++++ 3 files changed, 36 insertions(+) create mode 100644 test/fixtures/xml_external_entities.xml create mode 100644 test/pleroma/web/xml_test.exs diff --git a/test/fixtures/xml_external_entities.xml b/test/fixtures/xml_external_entities.xml new file mode 100644 index 000000000..d5ff87134 --- /dev/null +++ b/test/fixtures/xml_external_entities.xml @@ -0,0 +1,3 @@ + + ]> +&xxe; diff --git a/test/pleroma/web/web_finger_test.exs b/test/pleroma/web/web_finger_test.exs index fafef54fe..be5e08776 100644 --- a/test/pleroma/web/web_finger_test.exs +++ b/test/pleroma/web/web_finger_test.exs @@ -180,5 +180,28 @@ defmodule Pleroma.Web.WebFingerTest do {:ok, _data} = WebFinger.finger("pekorino@pawoo.net") end + + test "refuses to process XML remote entities" do + Tesla.Mock.mock(fn + %{ + url: "https://pawoo.net/.well-known/webfinger?resource=acct:pekorino@pawoo.net" + } -> + {:ok, + %Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/xml_external_entities.xml"), + headers: [{"content-type", "application/xrd+xml"}] + }} + + %{url: "https://pawoo.net/.well-known/host-meta"} -> + {:ok, + %Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/tesla_mock/pawoo.net_host_meta") + }} + end) + + assert :error = WebFinger.finger("pekorino@pawoo.net") + end end end diff --git a/test/pleroma/web/xml_test.exs b/test/pleroma/web/xml_test.exs new file mode 100644 index 000000000..89d4709b6 --- /dev/null +++ b/test/pleroma/web/xml_test.exs @@ -0,0 +1,10 @@ +defmodule Pleroma.Web.XMLTest do + use Pleroma.DataCase, async: true + + alias Pleroma.Web.XML + + test "refuses to load external entities from XML" do + data = File.read!("test/fixtures/xml_external_entities.xml") + assert(:error == XML.parse_document(data)) + end +end From 6d48b0f1a93a5a44b95497063e885342240fbc27 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 4 Aug 2023 22:44:09 -0400 Subject: [PATCH 20/27] Document and test that XXE processing is disabled https://vuln.be/post/xxe-in-erlang-and-elixir/ --- changelog.d/akkoma-xml-remote-entities.security | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/akkoma-xml-remote-entities.security diff --git a/changelog.d/akkoma-xml-remote-entities.security b/changelog.d/akkoma-xml-remote-entities.security new file mode 100644 index 000000000..b3c86bee1 --- /dev/null +++ b/changelog.d/akkoma-xml-remote-entities.security @@ -0,0 +1 @@ +Restrict XML parser from processing external entitites (XXE) From 4099ddb3dc5840fa10cff743d87464acf7898a80 Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Sat, 5 Aug 2023 08:27:42 +0200 Subject: [PATCH 21/27] Mergeback release 2.5.4 --- CHANGELOG.md | 5 +++++ changelog.d/akkoma-xml-remote-entities.security | 2 +- mix.exs | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87e9c5298..65acfad3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Removed - BREAKING: Support for passwords generated with `crypt(3)` (Gnu Social migration artifact) +## 2.5.4 + +## Security +- Fix XML External Entity (XXE) loading vulnerability allowing to fetch arbitary files from the server's filesystem + ## 2.5.3 ### Security diff --git a/changelog.d/akkoma-xml-remote-entities.security b/changelog.d/akkoma-xml-remote-entities.security index b3c86bee1..5e6725e5b 100644 --- a/changelog.d/akkoma-xml-remote-entities.security +++ b/changelog.d/akkoma-xml-remote-entities.security @@ -1 +1 @@ -Restrict XML parser from processing external entitites (XXE) +Fix XML External Entity (XXE) loading vulnerability allowing to fetch arbitary files from the server's filesystem diff --git a/mix.exs b/mix.exs index 115e7113f..b071e7c7b 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Pleroma.Mixfile do def project do [ app: :pleroma, - version: version("2.5.53"), + version: version("2.5.54"), elixir: "~> 1.11", elixirc_paths: elixirc_paths(Mix.env()), compilers: [:phoenix] ++ Mix.compilers(), From 48b1e9bdc7382ec6ef33e95f2bd8674ae92f17b2 Mon Sep 17 00:00:00 2001 From: mae Date: Sat, 5 Aug 2023 14:13:49 +0200 Subject: [PATCH 22/27] Completely disable xml entity resolution --- .../disable-xml-entity-resolution.security | 1 + lib/pleroma/web/xml.ex | 2 +- test/fixtures/xml_billion_laughs.xml | 15 +++++++++++++++ test/pleroma/web/xml_test.exs | 5 +++++ 4 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 changelog.d/disable-xml-entity-resolution.security create mode 100644 test/fixtures/xml_billion_laughs.xml diff --git a/changelog.d/disable-xml-entity-resolution.security b/changelog.d/disable-xml-entity-resolution.security new file mode 100644 index 000000000..db8e12f67 --- /dev/null +++ b/changelog.d/disable-xml-entity-resolution.security @@ -0,0 +1 @@ +Disable XML entity resolution completely to fix a dos vulnerability diff --git a/lib/pleroma/web/xml.ex b/lib/pleroma/web/xml.ex index 380a80ab8..64329e4ba 100644 --- a/lib/pleroma/web/xml.ex +++ b/lib/pleroma/web/xml.ex @@ -31,7 +31,7 @@ defmodule Pleroma.Web.XML do |> :binary.bin_to_list() |> :xmerl_scan.string( quiet: true, - fetch_fun: fn _, _ -> raise "Resolving external entities not supported" end + allow_entities: false ) {:ok, doc} diff --git a/test/fixtures/xml_billion_laughs.xml b/test/fixtures/xml_billion_laughs.xml new file mode 100644 index 000000000..75fb24cae --- /dev/null +++ b/test/fixtures/xml_billion_laughs.xml @@ -0,0 +1,15 @@ + + + + + + + + + + + + +]> +&lol9; diff --git a/test/pleroma/web/xml_test.exs b/test/pleroma/web/xml_test.exs index 89d4709b6..49306430b 100644 --- a/test/pleroma/web/xml_test.exs +++ b/test/pleroma/web/xml_test.exs @@ -3,6 +3,11 @@ defmodule Pleroma.Web.XMLTest do alias Pleroma.Web.XML + test "refuses to parse any entities from XML" do + data = File.read!("test/fixtures/xml_billion_laughs.xml") + assert(:error == XML.parse_document(data)) + end + test "refuses to load external entities from XML" do data = File.read!("test/fixtures/xml_external_entities.xml") assert(:error == XML.parse_document(data)) From c298e0165c29b30380466910bb328964a7264c4c Mon Sep 17 00:00:00 2001 From: Cat pony Black Date: Sat, 5 Aug 2023 13:10:35 +0200 Subject: [PATCH 23/27] Fix config ownership in dockerfile to pass restriction test --- Dockerfile | 2 +- changelog.d/dockerfile-config-perms.fix | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/dockerfile-config-perms.fix diff --git a/Dockerfile b/Dockerfile index 310d18104..72c227b76 100644 --- a/Dockerfile +++ b/Dockerfile @@ -49,7 +49,7 @@ USER pleroma COPY --from=build --chown=pleroma:0 /release ${HOME} -COPY ./config/docker.exs /etc/pleroma/config.exs +COPY --chown=pleroma --chmod=640 ./config/docker.exs /etc/pleroma/config.exs COPY ./docker-entrypoint.sh ${HOME} EXPOSE 4000 diff --git a/changelog.d/dockerfile-config-perms.fix b/changelog.d/dockerfile-config-perms.fix new file mode 100644 index 000000000..49ea5becb --- /dev/null +++ b/changelog.d/dockerfile-config-perms.fix @@ -0,0 +1 @@ +- Fix config ownership in dockerfile to pass restriction test From d838d1990bf23d452c1cc830629e42e51dbd7047 Mon Sep 17 00:00:00 2001 From: Haelwenn Date: Wed, 16 Aug 2023 13:34:32 +0000 Subject: [PATCH 24/27] Apply lanodan's suggestion(s) to 1 file(s) --- lib/pleroma/web/plugs/http_security_plug.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/plugs/http_security_plug.ex b/lib/pleroma/web/plugs/http_security_plug.ex index b3dc8a3a6..a3166bc96 100644 --- a/lib/pleroma/web/plugs/http_security_plug.ex +++ b/lib/pleroma/web/plugs/http_security_plug.ex @@ -93,7 +93,7 @@ defmodule Pleroma.Web.Plugs.HTTPSecurityPlug do img_src = "img-src 'self' data: blob:" media_src = "media-src 'self'" - connect_src = ["connect-src 'self' blob: ", ?\s, websocket_url] + connect_src = ["connect-src 'self' blob: ", static_url, ?\s, websocket_url] # Strict multimedia CSP enforcement only when MediaProxy is enabled {img_src, media_src, connect_src} = From 3d09bc320efcc68beb9b57fba23b6b9f3dc17905 Mon Sep 17 00:00:00 2001 From: tusooa Date: Wed, 30 Aug 2023 20:34:16 -0400 Subject: [PATCH 25/27] Make lint happy --- lib/pleroma/web/plugs/http_security_plug.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/plugs/http_security_plug.ex b/lib/pleroma/web/plugs/http_security_plug.ex index a3166bc96..5093414c4 100644 --- a/lib/pleroma/web/plugs/http_security_plug.ex +++ b/lib/pleroma/web/plugs/http_security_plug.ex @@ -100,6 +100,7 @@ defmodule Pleroma.Web.Plugs.HTTPSecurityPlug do if Config.get([:media_proxy, :enabled]) && !Config.get([:media_proxy, :proxy_opts, :redirect_on_failure]) do sources = build_csp_multimedia_source_list() + { [img_src, sources], [media_src, sources], @@ -113,7 +114,6 @@ defmodule Pleroma.Web.Plugs.HTTPSecurityPlug do } end - connect_src = if Config.get(:env) == :dev do [connect_src, " http://localhost:3035/"] From 3c5ecca37718a1eba05be1f379b8f47362079c65 Mon Sep 17 00:00:00 2001 From: tusooa Date: Wed, 30 Aug 2023 20:37:45 -0400 Subject: [PATCH 26/27] Skip changelog --- changelog.d/lint.skip | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 changelog.d/lint.skip diff --git a/changelog.d/lint.skip b/changelog.d/lint.skip new file mode 100644 index 000000000..e69de29bb From 1afde067b12ad0062c1820091ea9b0a680819281 Mon Sep 17 00:00:00 2001 From: Mint Date: Sat, 2 Sep 2023 01:43:25 +0300 Subject: [PATCH 27/27] CommonAPI: Prevent users from accessing media of other users --- .../check-attachment-attribution.security | 1 + lib/pleroma/scheduled_activity.ex | 6 ++- lib/pleroma/web/common_api.ex | 12 ++++++ lib/pleroma/web/common_api/activity_draft.ex | 2 +- lib/pleroma/web/common_api/utils.ex | 27 ++++++------ test/pleroma/web/common_api/utils_test.exs | 43 +++++++++++++------ test/pleroma/web/common_api_test.exs | 18 ++++++++ .../views/scheduled_activity_view_test.exs | 2 +- .../chat_message_reference_view_test.exs | 2 +- 9 files changed, 82 insertions(+), 31 deletions(-) create mode 100644 changelog.d/check-attachment-attribution.security diff --git a/changelog.d/check-attachment-attribution.security b/changelog.d/check-attachment-attribution.security new file mode 100644 index 000000000..e0e46525b --- /dev/null +++ b/changelog.d/check-attachment-attribution.security @@ -0,0 +1 @@ +CommonAPI: Prevent users from accessing media of other users by creating a status with reused attachment ID diff --git a/lib/pleroma/scheduled_activity.ex b/lib/pleroma/scheduled_activity.ex index a7be58512..0ed51ad07 100644 --- a/lib/pleroma/scheduled_activity.ex +++ b/lib/pleroma/scheduled_activity.ex @@ -40,7 +40,11 @@ defmodule Pleroma.ScheduledActivity do %{changes: %{params: %{"media_ids" => media_ids} = params}} = changeset ) when is_list(media_ids) do - media_attachments = Utils.attachments_from_ids(%{media_ids: media_ids}) + media_attachments = + Utils.attachments_from_ids( + %{media_ids: media_ids}, + User.get_cached_by_id(changeset.data.user_id) + ) params = params diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex index 77b3fa5d2..82c7f70d2 100644 --- a/lib/pleroma/web/common_api.ex +++ b/lib/pleroma/web/common_api.ex @@ -33,6 +33,7 @@ defmodule Pleroma.Web.CommonAPI do def post_chat_message(%User{} = user, %User{} = recipient, content, opts \\ []) do with maybe_attachment <- opts[:media_id] && Object.get_by_id(opts[:media_id]), + :ok <- validate_chat_attachment_attribution(maybe_attachment, user), :ok <- validate_chat_content_length(content, !!maybe_attachment), {_, {:ok, chat_message_data, _meta}} <- {:build_object, @@ -71,6 +72,17 @@ defmodule Pleroma.Web.CommonAPI do text end + defp validate_chat_attachment_attribution(nil, _), do: :ok + + defp validate_chat_attachment_attribution(attachment, user) do + with :ok <- Object.authorize_access(attachment, user) do + :ok + else + e -> + e + end + end + defp validate_chat_content_length(_, true), do: :ok defp validate_chat_content_length(nil, false), do: {:error, :no_content} diff --git a/lib/pleroma/web/common_api/activity_draft.ex b/lib/pleroma/web/common_api/activity_draft.ex index 9af635da8..63ed48a27 100644 --- a/lib/pleroma/web/common_api/activity_draft.ex +++ b/lib/pleroma/web/common_api/activity_draft.ex @@ -111,7 +111,7 @@ defmodule Pleroma.Web.CommonAPI.ActivityDraft do end defp attachments(%{params: params} = draft) do - attachments = Utils.attachments_from_ids(params) + attachments = Utils.attachments_from_ids(params, draft.user) draft = %__MODULE__{draft | attachments: attachments} case Utils.validate_attachments_count(attachments) do diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex index b9fe0224c..0f394e951 100644 --- a/lib/pleroma/web/common_api/utils.ex +++ b/lib/pleroma/web/common_api/utils.ex @@ -23,21 +23,21 @@ defmodule Pleroma.Web.CommonAPI.Utils do require Logger require Pleroma.Constants - def attachments_from_ids(%{media_ids: ids, descriptions: desc}) do - attachments_from_ids_descs(ids, desc) + def attachments_from_ids(%{media_ids: ids, descriptions: desc}, user) do + attachments_from_ids_descs(ids, desc, user) end - def attachments_from_ids(%{media_ids: ids}) do - attachments_from_ids_no_descs(ids) + def attachments_from_ids(%{media_ids: ids}, user) do + attachments_from_ids_no_descs(ids, user) end - def attachments_from_ids(_), do: [] + def attachments_from_ids(_, _), do: [] - def attachments_from_ids_no_descs([]), do: [] + def attachments_from_ids_no_descs([], _), do: [] - def attachments_from_ids_no_descs(ids) do + def attachments_from_ids_no_descs(ids, user) do Enum.map(ids, fn media_id -> - case get_attachment(media_id) do + case get_attachment(media_id, user) do %Object{data: data} -> data _ -> nil end @@ -45,22 +45,23 @@ defmodule Pleroma.Web.CommonAPI.Utils do |> Enum.reject(&is_nil/1) end - def attachments_from_ids_descs([], _), do: [] + def attachments_from_ids_descs([], _, _), do: [] - def attachments_from_ids_descs(ids, descs_str) do + def attachments_from_ids_descs(ids, descs_str, user) do {_, descs} = Jason.decode(descs_str) Enum.map(ids, fn media_id -> - with %Object{data: data} <- get_attachment(media_id) do + with %Object{data: data} <- get_attachment(media_id, user) do Map.put(data, "name", descs[media_id]) end end) |> Enum.reject(&is_nil/1) end - defp get_attachment(media_id) do + defp get_attachment(media_id, user) do with %Object{data: data} = object <- Repo.get(Object, media_id), - %{"type" => type} when type in Pleroma.Constants.upload_object_types() <- data do + %{"type" => type} when type in Pleroma.Constants.upload_object_types() <- data, + :ok <- Object.authorize_access(object, user) do object else _ -> nil diff --git a/test/pleroma/web/common_api/utils_test.exs b/test/pleroma/web/common_api/utils_test.exs index ca5b92683..4ce039d64 100644 --- a/test/pleroma/web/common_api/utils_test.exs +++ b/test/pleroma/web/common_api/utils_test.exs @@ -586,46 +586,61 @@ defmodule Pleroma.Web.CommonAPI.UtilsTest do end end - describe "attachments_from_ids_descs/2" do + describe "attachments_from_ids_descs/3" do test "returns [] when attachment ids is empty" do - assert Utils.attachments_from_ids_descs([], "{}") == [] + assert Utils.attachments_from_ids_descs([], "{}", nil) == [] end test "returns list attachments with desc" do - object = insert(:attachment) + user = insert(:user) + object = insert(:attachment, %{user: user}) desc = Jason.encode!(%{object.id => "test-desc"}) - assert Utils.attachments_from_ids_descs(["#{object.id}", "34"], desc) == [ + assert Utils.attachments_from_ids_descs(["#{object.id}", "34"], desc, user) == [ Map.merge(object.data, %{"name" => "test-desc"}) ] end end - describe "attachments_from_ids/1" do + describe "attachments_from_ids/2" do test "returns attachments with descs" do - object = insert(:attachment) + user = insert(:user) + object = insert(:attachment, %{user: user}) desc = Jason.encode!(%{object.id => "test-desc"}) - assert Utils.attachments_from_ids(%{ - media_ids: ["#{object.id}"], - descriptions: desc - }) == [ + assert Utils.attachments_from_ids( + %{ + media_ids: ["#{object.id}"], + descriptions: desc + }, + user + ) == [ Map.merge(object.data, %{"name" => "test-desc"}) ] end test "returns attachments without descs" do - object = insert(:attachment) - assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}) == [object.data] + user = insert(:user) + object = insert(:attachment, %{user: user}) + assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}, user) == [object.data] end test "returns [] when not pass media_ids" do - assert Utils.attachments_from_ids(%{}) == [] + assert Utils.attachments_from_ids(%{}, nil) == [] + end + + test "returns [] when media_ids not belong to current user" do + user = insert(:user) + user2 = insert(:user) + + object = insert(:attachment, %{user: user}) + + assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}, user2) == [] end test "checks that the object is of upload type" do object = insert(:note) - assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}) == [] + assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}, nil) == [] end end diff --git a/test/pleroma/web/common_api_test.exs b/test/pleroma/web/common_api_test.exs index 968e11a14..0d76d6581 100644 --- a/test/pleroma/web/common_api_test.exs +++ b/test/pleroma/web/common_api_test.exs @@ -279,6 +279,24 @@ defmodule Pleroma.Web.CommonAPITest do assert {:reject, "[KeywordPolicy] Matches with rejected keyword"} == CommonAPI.post_chat_message(author, recipient, "GNO/Linux") end + + test "it reject messages with attachments not belonging to user" do + author = insert(:user) + not_author = insert(:user) + recipient = author + + attachment = insert(:attachment, %{user: not_author}) + + {:error, message} = + CommonAPI.post_chat_message( + author, + recipient, + "123", + media_id: attachment.id + ) + + assert message == :forbidden + end end describe "unblocking" do diff --git a/test/pleroma/web/mastodon_api/views/scheduled_activity_view_test.exs b/test/pleroma/web/mastodon_api/views/scheduled_activity_view_test.exs index e5e510d33..07a65a3bc 100644 --- a/test/pleroma/web/mastodon_api/views/scheduled_activity_view_test.exs +++ b/test/pleroma/web/mastodon_api/views/scheduled_activity_view_test.exs @@ -48,7 +48,7 @@ defmodule Pleroma.Web.MastodonAPI.ScheduledActivityViewTest do id: to_string(scheduled_activity.id), media_attachments: %{media_ids: [upload.id]} - |> Utils.attachments_from_ids() + |> Utils.attachments_from_ids(user) |> Enum.map(&StatusView.render("attachment.json", %{attachment: &1})), params: %{ in_reply_to_id: to_string(activity.id), diff --git a/test/pleroma/web/pleroma_api/views/chat_message_reference_view_test.exs b/test/pleroma/web/pleroma_api/views/chat_message_reference_view_test.exs index 017c9c5c0..7ab3f5acd 100644 --- a/test/pleroma/web/pleroma_api/views/chat_message_reference_view_test.exs +++ b/test/pleroma/web/pleroma_api/views/chat_message_reference_view_test.exs @@ -24,7 +24,7 @@ defmodule Pleroma.Web.PleromaAPI.ChatMessageReferenceViewTest do filename: "an_image.jpg" } - {:ok, upload} = ActivityPub.upload(file, actor: user.ap_id) + {:ok, upload} = ActivityPub.upload(file, actor: recipient.ap_id) {:ok, activity} = CommonAPI.post_chat_message(user, recipient, "kippis :firefox:", idempotency_key: "123")