From 7d94476dd6e46071cf5b8662055a109f9ef904ef Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Tue, 20 Feb 2024 08:45:48 +0100 Subject: [PATCH 1/2] StealEmojiPolicy: Sanitize shortcodes Closes: https://git.pleroma.social/pleroma/pleroma/-/issues/3245 --- .../activity_pub/mrf/steal_emoji_policy.ex | 2 ++ .../mrf/steal_emoji_policy_test.exs | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex b/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex index 45b2d4ca7..3e1dc7d43 100644 --- a/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex @@ -34,6 +34,7 @@ defp steal_emoji({shortcode, url}, emoji_dir_path) do |> Path.basename() |> Path.extname() + shortcode = Path.basename(shortcode) file_path = Path.join(emoji_dir_path, shortcode <> (extension || ".png")) case File.write(file_path, response.body) do @@ -76,6 +77,7 @@ def filter(%{"object" => %{"emoji" => foreign_emojis, "actor" => actor}} = messa new_emojis = foreign_emojis |> Enum.reject(fn {shortcode, _url} -> shortcode in installed_emoji end) + |> Enum.reject(fn {shortcode, _url} -> String.contains?(shortcode, ["/", "\\"]) end) |> Enum.filter(fn {shortcode, _url} -> reject_emoji? = [:mrf_steal_emoji, :rejected_shortcodes] diff --git a/test/pleroma/web/activity_pub/mrf/steal_emoji_policy_test.exs b/test/pleroma/web/activity_pub/mrf/steal_emoji_policy_test.exs index b0a7e8993..59baa3a43 100644 --- a/test/pleroma/web/activity_pub/mrf/steal_emoji_policy_test.exs +++ b/test/pleroma/web/activity_pub/mrf/steal_emoji_policy_test.exs @@ -60,6 +60,32 @@ test "Steals emoji on unknown shortcode from allowed remote host", %{ |> File.exists?() end + test "rejects invalid shortcodes", %{path: path} do + message = %{ + "type" => "Create", + "object" => %{ + "emoji" => [{"fired/fox", "https://example.org/emoji/firedfox"}], + "actor" => "https://example.org/users/admin" + } + } + + fullpath = Path.join(path, "fired/fox.png") + + Tesla.Mock.mock(fn %{method: :get, url: "https://example.org/emoji/firedfox"} -> + %Tesla.Env{status: 200, body: File.read!("test/fixtures/image.jpg")} + end) + + clear_config(:mrf_steal_emoji, hosts: ["example.org"], size_limit: 284_468) + + refute "firedfox" in installed() + refute File.exists?(path) + + assert {:ok, _message} = StealEmojiPolicy.filter(message) + + refute "fired/fox" in installed() + refute File.exists?(fullpath) + end + test "reject regex shortcode", %{message: message} do refute "firedfox" in installed() From b387f4a1c1ff02573f16de0b25403cf501afc3b4 Mon Sep 17 00:00:00 2001 From: Erin Shepherd Date: Tue, 20 Feb 2024 11:31:29 +0100 Subject: [PATCH 2/2] Don't steal emoji who's shortcodes have dots or colons in their name Mastodon at the very least seems to prevent the creation of emoji with dots in their name (and refuses to accept them in federation). It feels like being cautious in what we accept is reasonable here. Colons are the emoji separator and so obviously should be blocked. Perhaps instead of filtering out things like this we should just do a regex match on `[a-zA-Z0-9_-]`? But that's plausibly a decision for another day Perhaps we should also have a centralised "is this a valid emoji shortcode?" function --- lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex b/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex index 3e1dc7d43..6c43afa58 100644 --- a/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex @@ -77,7 +77,7 @@ def filter(%{"object" => %{"emoji" => foreign_emojis, "actor" => actor}} = messa new_emojis = foreign_emojis |> Enum.reject(fn {shortcode, _url} -> shortcode in installed_emoji end) - |> Enum.reject(fn {shortcode, _url} -> String.contains?(shortcode, ["/", "\\"]) end) + |> Enum.reject(fn {shortcode, _url} -> String.contains?(shortcode, ["/", "\\", ".", ":"]) end) |> Enum.filter(fn {shortcode, _url} -> reject_emoji? = [:mrf_steal_emoji, :rejected_shortcodes]