From a8c6c780b4e0d0db9a393ff6bb6a3b904ad94269 Mon Sep 17 00:00:00 2001 From: Oneric Date: Thu, 7 Mar 2024 23:35:05 +0100 Subject: [PATCH] StealEmoji: use Content-Type and reject non-images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit E.g. *key’s emoji URLs typically don’t have file extensions, but until now we just slapped ".png" at its end hoping for the best. Furthermore, this gives us a chance to actually reject non-images, which before was not feasible exatly due to those extension-less URLs --- .../activity_pub/mrf/steal_emoji_policy.ex | 26 ++++++---- .../mrf/steal_emoji_policy_test.exs | 51 +++++++++++++++++-- 2 files changed, 62 insertions(+), 15 deletions(-) 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 c743ac3a2..9b2fa3020 100644 --- a/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex @@ -33,16 +33,9 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicy do !valid_shortcode? or rejected_shortcode? or emoji_installed? end - defp steal_emoji(%{} = response, {shortcode, url}, emoji_dir_path) do - extension = - url - |> URI.parse() - |> Map.get(:path) - |> Path.basename() - |> Path.extname() - + defp steal_emoji(%{} = response, {shortcode, extension}, emoji_dir_path) do shortcode = Path.basename(shortcode) - file_path = Path.join(emoji_dir_path, shortcode <> (extension || ".png")) + file_path = Path.join(emoji_dir_path, shortcode <> "." <> extension) case File.write(file_path, response.body) do :ok -> @@ -54,14 +47,25 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicy do end end + defp get_extension_if_safe(response) do + content_type = + :proplists.get_value("content-type", response.headers, MIME.from_path(response.url)) + + case content_type do + "image/" <> _ -> List.first(MIME.extensions(content_type)) + _ -> nil + end + end + defp maybe_steal_emoji({shortcode, url}, emoji_dir_path) do url = Pleroma.Web.MediaProxy.url(url) with {:ok, %{status: status} = response} when status in 200..299 <- Pleroma.HTTP.get(url) do size_limit = Config.get([:mrf_steal_emoji, :size_limit], 50_000) + extension = get_extension_if_safe(response) - if byte_size(response.body) <= size_limit do - steal_emoji(response, {shortcode, url}, emoji_dir_path) + if byte_size(response.body) <= size_limit and extension do + steal_emoji(response, {shortcode, extension}, emoji_dir_path) else Logger.debug( "MRF.StealEmojiPolicy: :#{shortcode}: at #{url} (#{byte_size(response.body)} B) over size limit (#{size_limit} B)" 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 59baa3a43..b0f6d351c 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 @@ -45,7 +45,11 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicyTest do refute File.exists?(path) Tesla.Mock.mock(fn %{method: :get, url: "https://example.org/emoji/firedfox.png"} -> - %Tesla.Env{status: 200, body: File.read!("test/fixtures/image.jpg")} + %Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/image.jpg"), + url: "https://example.org/emoji/firedfox.png" + } end) clear_config(:mrf_steal_emoji, hosts: ["example.org"], size_limit: 284_468) @@ -72,7 +76,11 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicyTest do 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")} + %Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/image.jpg"), + url: "https://example.org/emoji/firedfox.png" + } end) clear_config(:mrf_steal_emoji, hosts: ["example.org"], size_limit: 284_468) @@ -86,6 +94,36 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicyTest do refute File.exists?(fullpath) end + test "prefers content-type header for extension", %{path: path} do + message = %{ + "type" => "Create", + "object" => %{ + "emoji" => [{"firedfox", "https://example.org/emoji/firedfox.fud"}], + "actor" => "https://example.org/users/admin" + } + } + + Tesla.Mock.mock(fn %{method: :get, url: "https://example.org/emoji/firedfox.fud"} -> + %Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/image.jpg"), + url: "https://example.org/emoji/firedfox.wevp", + headers: [{"content-type", "image/gif"}] + } + end) + + clear_config(:mrf_steal_emoji, hosts: ["example.org"], size_limit: 284_468) + + assert {:ok, _message} = StealEmojiPolicy.filter(message) + + assert "firedfox" in installed() + assert File.exists?(path) + + assert path + |> Path.join("firedfox.gif") + |> File.exists?() + end + test "reject regex shortcode", %{message: message} do refute "firedfox" in installed() @@ -118,7 +156,11 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicyTest do refute "firedfox" in installed() Tesla.Mock.mock(fn %{method: :get, url: "https://example.org/emoji/firedfox.png"} -> - %Tesla.Env{status: 200, body: File.read!("test/fixtures/image.jpg")} + %Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/image.jpg"), + url: "https://example.org/emoji/firedfox.png" + } end) clear_config(:mrf_steal_emoji, hosts: ["example.org"], size_limit: 50_000) @@ -132,7 +174,8 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicyTest do refute "firedfox" in installed() Tesla.Mock.mock(fn %{method: :get, url: "https://example.org/emoji/firedfox.png"} -> - {:ok, %Tesla.Env{status: 404, body: "Not found"}} + {:ok, + %Tesla.Env{status: 404, body: "Not found", url: "https://example.org/emoji/firedfox.png"}} end) clear_config(:mrf_steal_emoji, hosts: ["example.org"], size_limit: 284_468)