From 066d5b48ed463a14129e9015211c769ff61affeb Mon Sep 17 00:00:00 2001 From: Oneric Date: Mon, 10 Mar 2025 18:45:12 +0100 Subject: [PATCH] Fix Content-Type sanitisation for emoji and local uploads This was accidentally broken in c8e0f7848bd72434a30e450a5af8c356edb7fbcb due to a one-letter mistake in the plug option name and an absence of tests. Therefore it was once again possible to serve e.g. Javascript or CSS payloads via uploads and emoji. However due to other protections it was still NOT possible for anyone to serve any payload with an ActivityPub Content-Type. With the CSP policy hardening from previous JS payload exloits predating the Content-Type sanitisation, there is currently no known way of abusing this weakened Content-Type sanitisation, but should be fixed regardless. This commit fixes the option name and adds tests to ensure such a regression doesn't occur again in the future. Reported-by: Lain Soykaf --- lib/pleroma/web/plugs/instance_static.ex | 2 +- lib/pleroma/web/plugs/uploaded_media.ex | 2 +- .../web/content_type_sanitisation_test.exs | 66 +++++++++++++++ .../web/content_type_sanitisation_template.ex | 81 +++++++++++++++++++ 4 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 test/pleroma/web/content_type_sanitisation_test.exs create mode 100644 test/support/web/content_type_sanitisation_template.ex diff --git a/lib/pleroma/web/plugs/instance_static.ex b/lib/pleroma/web/plugs/instance_static.ex index ec188986b..5456634a1 100644 --- a/lib/pleroma/web/plugs/instance_static.ex +++ b/lib/pleroma/web/plugs/instance_static.ex @@ -62,7 +62,7 @@ defp call_static(%{request_path: request_path} = conn, opts, from) do opts = opts |> Map.put(:from, from) - |> Map.put(:content_type, false) + |> Map.put(:content_types, false) conn |> set_static_content_type(request_path) diff --git a/lib/pleroma/web/plugs/uploaded_media.ex b/lib/pleroma/web/plugs/uploaded_media.ex index 357fcb432..15d6190df 100644 --- a/lib/pleroma/web/plugs/uploaded_media.ex +++ b/lib/pleroma/web/plugs/uploaded_media.ex @@ -88,7 +88,7 @@ defp get_media(conn, {:static_dir, directory}, opts) do Map.get(opts, :static_plug_opts) |> Map.put(:at, [@path]) |> Map.put(:from, directory) - |> Map.put(:content_type, false) + |> Map.put(:content_types, false) conn = conn diff --git a/test/pleroma/web/content_type_sanitisation_test.exs b/test/pleroma/web/content_type_sanitisation_test.exs new file mode 100644 index 000000000..6aa335b6e --- /dev/null +++ b/test/pleroma/web/content_type_sanitisation_test.exs @@ -0,0 +1,66 @@ +# Akkoma: Magically expressive social media +# Copyright © 2025 Akkoma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ContentTypeSanitisationTest do + use Pleroma.Web.ConnCase, async: true + + alias Pleroma.Web.ContentTypeSanitisationTemplate, as: Template + + require Template + + defp create_file(path, body) do + File.write!(path, body) + on_exit(fn -> File.rm(path) end) + end + + defp upload_dir(), + do: Path.join(Pleroma.Uploaders.Local.upload_path(), "test_StaticPlugSanitisationTest") + + defp create_upload(subpath, body) do + Path.join(upload_dir(), subpath) + |> create_file(body) + + "/media/test_StaticPlugSanitisationTest/#{subpath}" + end + + defp emoji_dir(), + do: + Path.join( + Pleroma.Config.get!([:instance, :static_dir]), + "emoji/test_StaticPlugSanitisationTest" + ) + + defp create_emoji(subpath, body) do + Path.join(emoji_dir(), subpath) + |> create_file(body) + + "/emoji/test_StaticPlugSanitisationTest/#{subpath}" + end + + setup_all do + File.mkdir_p(upload_dir()) + File.mkdir_p(emoji_dir()) + + on_exit(fn -> + File.rm_rf!(upload_dir()) + File.rm_rf!(emoji_dir()) + end) + end + + describe "sanitises Content-Type of local uploads" do + Template.do_all_common_tests(&create_upload/2) + + test "while preserving audio types" do + Template.do_audio_test(&create_upload/2, false) + end + end + + describe "sanitises Content-Type of emoji" do + Template.do_all_common_tests(&create_emoji/2) + + test "if audio type" do + Template.do_audio_test(&create_emoji/2, true) + end + end +end diff --git a/test/support/web/content_type_sanitisation_template.ex b/test/support/web/content_type_sanitisation_template.ex new file mode 100644 index 000000000..c785a0e87 --- /dev/null +++ b/test/support/web/content_type_sanitisation_template.ex @@ -0,0 +1,81 @@ +# Akkoma: Magically expressive social media +# Copyright © 2025 Akkoma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ContentTypeSanitisationTemplate do + defmacro do_test(create_fun, fname, body, content_type) do + quote do + url = unquote(create_fun).(unquote(fname), unquote(body)) + resp = get(build_conn(), url) + assert resp.status == 200 + + assert Enum.all?( + Plug.Conn.get_resp_header(resp, "content-type"), + fn e -> e == unquote(content_type) end + ) + end + end + + defmacro do_all_common_tests(create_fun) do + quote do + test "while preserving image types" do + unquote(__MODULE__).do_test( + unquote(create_fun), + "image.jpg", + File.read!("test/fixtures/image.jpg"), + "image/jpeg" + ) + end + + test "if ActivityPub type" do + # this already ought to be impossible from the configured MIME mapping, but let’s make sure + unquote(__MODULE__).do_test( + unquote(create_fun), + "ap.activity+json", + "{\"a\": \"b\"}", + "application/octet-stream" + ) + end + + test "if PDF type" do + unquote(__MODULE__).do_test( + unquote(create_fun), + "pdf.pdf", + "pdf stub", + "application/octet-stream" + ) + end + + test "if Javascript type" do + unquote(__MODULE__).do_test( + unquote(create_fun), + "script.js", + "alert('miaow');", + "application/octet-stream" + ) + end + + test "if CSS type" do + unquote(__MODULE__).do_test( + unquote(create_fun), + "script.js", + ".StatusBody {display: none;}", + "application/octet-stream" + ) + end + end + end + + defmacro do_audio_test(create_fun, sanitise \\ false) do + quote do + expected_type = if unquote(sanitise), do: "application/octet-stream", else: "audio/mpeg" + + unquote(__MODULE__).do_test( + unquote(create_fun), + "audio.mp3", + File.read!("test/fixtures/sound.mp3"), + expected_type + ) + end + end +end