From 5b337f952ac68ba94890092d09dd115c555919c4 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Wed, 6 Dec 2023 08:25:02 +0100 Subject: [PATCH 01/17] refactor(activitypub): handle failure finding public key in actor keys Signed-off-by: Thomas Citharel --- lib/federation/activity_pub/utils.ex | 21 +++++++----- .../activity_stream/converter/actor.ex | 34 +++++++++++++------ 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/lib/federation/activity_pub/utils.ex b/lib/federation/activity_pub/utils.ex index 51441fc05..38c9a574e 100644 --- a/lib/federation/activity_pub/utils.ex +++ b/lib/federation/activity_pub/utils.ex @@ -680,19 +680,22 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do @doc """ Converts PEM encoded keys to a public key representation """ - @spec pem_to_public_key_pem(String.t()) :: String.t() + @spec pem_to_public_key_pem(String.t()) :: String.t() | {:error, :no_publickey_found} def pem_to_public_key_pem(pem) do - public_key = pem_to_public_key(pem) - public_key = :public_key.pem_entry_encode(:RSAPublicKey, public_key) - :public_key.pem_encode([public_key]) + case :public_key.pem_decode(pem) do + [key_code] -> + public_key = pem_to_public_key(key_code) + public_key = :public_key.pem_entry_encode(:RSAPublicKey, public_key) + :public_key.pem_encode([public_key]) + + _ -> + {:error, :no_publickey_found} + end end @spec pem_to_public_key(String.t()) :: {:RSAPublicKey, any(), any()} - defp pem_to_public_key(pem) do - [key_code] = :public_key.pem_decode(pem) - key = :public_key.pem_entry_decode(key_code) - - case key do + defp pem_to_public_key(key_code) do + case :public_key.pem_entry_decode(key_code) do {:RSAPrivateKey, _, modulus, exponent, _, _, _, _, _, _, _} -> {:RSAPublicKey, modulus, exponent} diff --git a/lib/federation/activity_stream/converter/actor.ex b/lib/federation/activity_stream/converter/actor.ex index db9a4b91a..c44216116 100644 --- a/lib/federation/activity_stream/converter/actor.ex +++ b/lib/federation/activity_stream/converter/actor.ex @@ -112,19 +112,11 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Actor do }, "discoverable" => actor.visibility == :public, "openness" => actor.openness, - "manuallyApprovesFollowers" => actor.manually_approves_followers, - "publicKey" => %{ - "id" => "#{actor.url}#main-key", - "owner" => actor.url, - "publicKeyPem" => - if(is_nil(actor.domain) and not is_nil(actor.keys), - do: Utils.pem_to_public_key_pem(actor.keys), - else: actor.keys - ) - } + "manuallyApprovesFollowers" => actor.manually_approves_followers } actor_data + |> add_keys(actor) |> add_endpoints(actor) |> maybe_add_members(actor) |> maybe_add_avatar_picture(actor) @@ -132,6 +124,28 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Actor do |> maybe_add_physical_address(actor) end + @spec add_keys(map(), ActorModel.t()) :: map() + defp add_keys(actor_data, %ActorModel{} = actor) do + keys = + if is_nil(actor.domain) and not is_nil(actor.keys) do + case Utils.pem_to_public_key_pem(actor.keys) do + {:error, :no_publickey_found} -> + raise "No publickey found in private keys" + + public_key when is_binary(public_key) -> + public_key + end + else + actor.keys + end + + Map.put(actor_data, "publicKey", %{ + "id" => "#{actor.url}#main-key", + "owner" => actor.url, + "publicKeyPem" => keys + }) + end + defp add_endpoints(%{"endpoints" => endpoints} = actor_data, %ActorModel{} = actor) do new_endpoints = %{ "members" => actor.members_url, From 935799f123d22fe2ed0f61ffaad741eb0c4bcbca Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Wed, 6 Dec 2023 08:37:48 +0100 Subject: [PATCH 02/17] fix(front): fix editing group Signed-off-by: Thomas Citharel --- src/views/Group/GroupSettings.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/views/Group/GroupSettings.vue b/src/views/Group/GroupSettings.vue index d32d32cb5..64a0d5c70 100644 --- a/src/views/Group/GroupSettings.vue +++ b/src/views/Group/GroupSettings.vue @@ -261,7 +261,7 @@ const copyURL = async (): Promise => { onGroupResult(async ({ data }) => { if (!data) return; - editableGroup.value = data.group; + editableGroup.value = { ...data.group }; try { avatarFile.value = await buildFileFromIMedia(editableGroup.value?.avatar); bannerFile.value = await buildFileFromIMedia(editableGroup.value?.banner); From ded59bec276abb3611432d789445095295cde557 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Wed, 6 Dec 2023 08:47:28 +0100 Subject: [PATCH 03/17] fix(front): fix XSS because of bad operations when setting the group's summary Group summary (HTML) is properly sanitized by the backend, but for groups we did a special operation before setting the HTML in the Vue app. This is now removed Signed-off-by: Thomas Citharel --- src/components/Group/GroupCard.vue | 5 +---- src/components/Group/GroupMemberCard.vue | 5 ++--- src/utils/html.ts | 9 --------- 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/src/components/Group/GroupCard.vue b/src/components/Group/GroupCard.vue index 7ef9c2ed3..9b5151fc4 100644 --- a/src/components/Group/GroupCard.vue +++ b/src/components/Group/GroupCard.vue @@ -40,7 +40,7 @@
@@ -91,7 +91,6 @@ import { addressFullName } from "@/types/address.model"; import { useI18n } from "vue-i18n"; import AccountGroup from "vue-material-design-icons/AccountGroup.vue"; import Account from "vue-material-design-icons/Account.vue"; -import { htmlToText } from "@/utils/html"; import { computed } from "vue"; import LinkOrRouterLink from "../core/LinkOrRouterLink.vue"; @@ -108,8 +107,6 @@ const props = withDefaults( const { t } = useI18n({ useScope: "global" }); -const saneSummary = computed(() => htmlToText(props.group.summary ?? "")); - const isInternal = computed(() => { return props.isRemoteGroup && props.isLoggedIn === false; }); diff --git a/src/components/Group/GroupMemberCard.vue b/src/components/Group/GroupMemberCard.vue index 3693e38f9..8a3aa3f54 100644 --- a/src/components/Group/GroupMemberCard.vue +++ b/src/components/Group/GroupMemberCard.vue @@ -60,9 +60,9 @@
@@ -95,7 +95,6 @@ import DotsHorizontal from "vue-material-design-icons/DotsHorizontal.vue"; import AccountGroup from "vue-material-design-icons/AccountGroup.vue"; import AccountCircle from "vue-material-design-icons/AccountCircle.vue"; import Tag from "@/components/TagElement.vue"; -import { htmlToText } from "@/utils/html"; import { useI18n } from "vue-i18n"; defineProps<{ diff --git a/src/utils/html.ts b/src/utils/html.ts index e1baac0b8..084970f70 100644 --- a/src/utils/html.ts +++ b/src/utils/html.ts @@ -2,15 +2,6 @@ export function nl2br(text: string): string { return text.replace(/(?:\r\n|\r|\n)/g, "
"); } -export function htmlToText(html: string) { - const template = document.createElement("template"); - const trimmedHTML = html.trim(); - template.innerHTML = trimmedHTML; - const text = template.content.textContent; - template.remove(); - return text; -} - export const getValueFromMeta = (name: string): string | null => { const element = document.querySelector(`meta[name="${name}"]`); if (element && element.getAttribute("content")) { From ffff379d47cd75f63de217a4d7fb93e6d6ecbe73 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Wed, 6 Dec 2023 11:05:56 +0100 Subject: [PATCH 04/17] fix: always consider report content as text Report content was used as HTML in front-end and e-mails but wasn't sanitized as such. Signed-off-by: Thomas Citharel --- .../activity_stream/converter/flag.ex | 1 + lib/web/templates/email/report.html.heex | 2 +- src/components/Report/ReportCard.vue | 4 +- src/views/Moderation/ReportView.vue | 9 ++-- .../activity_pub/types/reports_test.exs | 41 +++++++++++++++++++ 5 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 test/federation/activity_pub/types/reports_test.exs diff --git a/lib/federation/activity_stream/converter/flag.ex b/lib/federation/activity_stream/converter/flag.ex index 347ee8d00..1d4944524 100644 --- a/lib/federation/activity_stream/converter/flag.ex +++ b/lib/federation/activity_stream/converter/flag.ex @@ -60,6 +60,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Flag do "actor" => Relay.get_actor().url, "id" => report.url, "content" => report.content, + "mediaType" => "text/plain", "object" => object } end diff --git a/lib/web/templates/email/report.html.heex b/lib/web/templates/email/report.html.heex index 0a8ad5bbe..002e81ac5 100644 --- a/lib/web/templates/email/report.html.heex +++ b/lib/web/templates/email/report.html.heex @@ -192,7 +192,7 @@ >

<%= gettext("Reasons for report") %>

- <%= @report.content |> raw %> + <%= @report.content %>

-
+
+ {{ report.content }} +
diff --git a/src/views/Moderation/ReportView.vue b/src/views/Moderation/ReportView.vue index 615c50686..0574dcf0a 100644 --- a/src/views/Moderation/ReportView.vue +++ b/src/views/Moderation/ReportView.vue @@ -216,11 +216,9 @@

{{ t("Unknown actor") }}

-
+
+ {{ report.content }} +

{{ t("No comment") }}

@@ -407,7 +405,6 @@ import { } from "@/types/actor"; import { DELETE_EVENT } from "@/graphql/event"; import uniq from "lodash/uniq"; -import { nl2br } from "@/utils/html"; import { DELETE_COMMENT } from "@/graphql/comment"; import { IComment } from "@/types/comment.model"; import { ActorType, AntiSpamFeedback, ReportStatusEnum } from "@/types/enums"; diff --git a/test/federation/activity_pub/types/reports_test.exs b/test/federation/activity_pub/types/reports_test.exs new file mode 100644 index 000000000..8c17fe731 --- /dev/null +++ b/test/federation/activity_pub/types/reports_test.exs @@ -0,0 +1,41 @@ +defmodule Mobilizon.Federation.ActivityPub.Types.ReportsTest do + use Mobilizon.DataCase + + import Mobilizon.Factory + + alias Mobilizon.Actors.Actor + alias Mobilizon.Federation.ActivityPub.Types.Reports + alias Mobilizon.Reports.Report + + describe "report creation" do + test "with XSS" do + %Actor{id: reporter_id} = insert(:actor) + %Actor{id: reported_id} = insert(:actor) + + content = + "hello " + + assert {:ok, %Report{content: saved_content}, _} = + Reports.flag(%{ + reporter_id: reporter_id, + reported_id: reported_id, + content: content + }) + + assert saved_content == "hello " + + content = + "<meta http-equiv=\"refresh\" content=\"0; url=http://example.com/\" />" + + assert {:ok, %Report{content: saved_content}, _} = + Reports.flag(%{ + reporter_id: reporter_id, + reported_id: reported_id, + content: content + }) + + assert saved_content == + "" + end + end +end From 2c12fbfd09d1bb501926dae8d1bd49a6cd0adb66 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Wed, 6 Dec 2023 11:18:05 +0100 Subject: [PATCH 05/17] fix(front): anonymous participant text is plain text, avoid using v-html It was using v-html when opening to "view more" Signed-off-by: Thomas Citharel --- src/filters/index.ts | 2 -- src/filters/utils.ts | 9 --------- src/utils/html.ts | 4 ---- src/views/Event/ParticipantsView.vue | 12 ++++++------ 4 files changed, 6 insertions(+), 21 deletions(-) delete mode 100644 src/filters/utils.ts diff --git a/src/filters/index.ts b/src/filters/index.ts index 941d69c7e..8d02384b6 100644 --- a/src/filters/index.ts +++ b/src/filters/index.ts @@ -1,4 +1,3 @@ -import nl2br from "@/filters/utils"; import { formatDateString, formatTimeString, @@ -11,6 +10,5 @@ export default { vue.filter("formatDateString", formatDateString); vue.filter("formatTimeString", formatTimeString); vue.filter("formatDateTimeString", formatDateTimeString); - vue.filter("nl2br", nl2br); }, }; diff --git a/src/filters/utils.ts b/src/filters/utils.ts deleted file mode 100644 index 58e91df6d..000000000 --- a/src/filters/utils.ts +++ /dev/null @@ -1,9 +0,0 @@ -/** - * New Line to
- * - * @param {string} str Input text - * @return {string} Filtered text - */ -export default function nl2br(str: string): string { - return `${str}`.replace(/([^>\r\n]?)(\r\n|\n\r|\r|\n)/g, "$1
"); -} diff --git a/src/utils/html.ts b/src/utils/html.ts index 084970f70..02b8763b4 100644 --- a/src/utils/html.ts +++ b/src/utils/html.ts @@ -1,7 +1,3 @@ -export function nl2br(text: string): string { - return text.replace(/(?:\r\n|\r|\n)/g, "
"); -} - export const getValueFromMeta = (name: string): string | null => { const element = document.querySelector(`meta[name="${name}"]`); if (element && element.getAttribute("content")) { diff --git a/src/views/Event/ParticipantsView.vue b/src/views/Event/ParticipantsView.vue index 7624f9202..e5f33aee0 100644 --- a/src/views/Event/ParticipantsView.vue +++ b/src/views/Event/ParticipantsView.vue @@ -189,16 +189,15 @@

{{ props.row.metadata.message }}

- +

{{ t("No message") }} @@ -212,7 +211,9 @@