From d6d838cbe83e8caf3e1fc67a81c3943e880ab290 Mon Sep 17 00:00:00 2001 From: Oneric Date: Sun, 10 Mar 2024 01:35:35 +0100 Subject: [PATCH] StealEmoji: check remote size before downloading To save on bandwith and avoid OOMs with large files. Ofc, this relies on the remote server (a) sending a content-length header and (b) being honest about the size. Common fedi servers seem to provide the header and (b) at least raises the required privilege of an malicious actor to a server infrastructure admin of an explicitly allowed host. A more complete defense which still works when faced with a malicious server requires changes in upstream Finch; see https://github.com/sneako/finch/issues/224 --- docs/docs/configuration/cheatsheet.md | 4 +- .../activity_pub/mrf/steal_emoji_policy.ex | 25 +++++++++- .../mrf/steal_emoji_policy_test.exs | 50 ++++++++++++++++++- 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/docs/docs/configuration/cheatsheet.md b/docs/docs/configuration/cheatsheet.md index 40d1319c7..c4259c6cf 100644 --- a/docs/docs/configuration/cheatsheet.md +++ b/docs/docs/configuration/cheatsheet.md @@ -236,7 +236,9 @@ config :pleroma, :mrf_user_allowlist, %{ #### :mrf_steal_emoji * `hosts`: List of hosts to steal emojis from * `rejected_shortcodes`: Regex-list of shortcodes to reject -* `size_limit`: File size limit (in bytes), checked before an emoji is saved to the disk +* `size_limit`: File size limit (in bytes), checked before download if possible (and remote server honest), + otherwise or again checked before saving emoji to the disk +* `download_unknown_size`: whether to download an emoji when the remote server doesn’t report its size in advance #### :mrf_activity_expiration 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 3a6eae3f2..26d3dc592 100644 --- a/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex @@ -13,6 +13,10 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicy do @pack_name "stolen" + # Config defaults + @size_limit 50_000 + @download_unknown_size false + defp create_pack() do with {:ok, pack} = Pack.create(@pack_name) do Pack.save_metadata( @@ -97,11 +101,28 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicy do end end + defp is_remote_size_within_limit?(url) do + with {:ok, %{status: status, headers: headers} = _response} when status in 200..299 <- + Pleroma.HTTP.request(:head, url, nil, [], []) do + content_length = :proplists.get_value("content-length", headers, nil) + size_limit = Config.get([:mrf_steal_emoji, :size_limit], @size_limit) + + accept_unknown = + Config.get([:mrf_steal_emoji, :download_unknown_size], @download_unknown_size) + + content_length <= size_limit or + (content_length == nil and accept_unknown) + else + _ -> false + end + end + defp maybe_steal_emoji({shortcode, url}) 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) + with {:remote_size, true} <- {:remote_size, is_remote_size_within_limit?(url)}, + {:ok, %{status: status} = response} when status in 200..299 <- Pleroma.HTTP.get(url) do + size_limit = Config.get([:mrf_steal_emoji, :size_limit], @size_limit) extension = get_extension_if_safe(response) if byte_size(response.body) <= size_limit and extension do 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 2103e8539..ba5087f1b 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 @@ -32,6 +32,14 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicyTest do ) do quote do Tesla.Mock.mock(fn + %{method: :head, url: unquote(url)} -> + %Tesla.Env{ + status: unquote(status), + body: nil, + url: unquote(url), + headers: unquote(headers) + } + %{method: :get, url: unquote(url)} -> %Tesla.Env{ status: unquote(status), @@ -46,7 +54,8 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicyTest do setup do clear_config(:mrf_steal_emoji, hosts: ["example.org"], - size_limit: 284_468 + size_limit: 284_468, + download_unknown_size: true ) emoji_path = [:instance, :static_dir] |> Config.get() |> Path.join("emoji/stolen") @@ -174,5 +183,44 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicyTest do refute "firedfox" in installed() end + test "reject unknown size", %{message: message} do + clear_config([:mrf_steal_emoji, :download_unknown_size], false) + mock_tesla() + + refute "firedfox" in installed() + + ExUnit.CaptureLog.capture_log(fn -> + assert {:ok, _message} = StealEmojiPolicy.filter(message) + end) =~ + "MRF.StealEmojiPolicy: Failed to fetch https://example.org/emoji/firedfox.png: {:remote_size, false}" + + refute "firedfox" in installed() + end + + test "reject too large content-size before download", %{message: message} do + clear_config([:mrf_steal_emoji, :download_unknown_size], false) + mock_tesla("https://example.org/emoji/firedfox.png", 200, [{"content-length", 2 ** 30}]) + + refute "firedfox" in installed() + + ExUnit.CaptureLog.capture_log(fn -> + assert {:ok, _message} = StealEmojiPolicy.filter(message) + end) =~ + "MRF.StealEmojiPolicy: Failed to fetch https://example.org/emoji/firedfox.png: {:remote_size, false}" + + refute "firedfox" in installed() + end + + test "accepts content-size below limit", %{message: message} do + clear_config([:mrf_steal_emoji, :download_unknown_size], false) + mock_tesla("https://example.org/emoji/firedfox.png", 200, [{"content-length", 2}]) + + refute "firedfox" in installed() + + assert {:ok, _message} = StealEmojiPolicy.filter(message) + + assert "firedfox" in installed() + end + defp installed, do: Emoji.get_all() |> Enum.map(fn {k, _} -> k end) end