diff --git a/changelog.d/3895.add b/changelog.d/3895.add new file mode 100644 index 0000000000..5b1737ebdc --- /dev/null +++ b/changelog.d/3895.add @@ -0,0 +1 @@ +Uploaded media content-type is scrubbed before serving the files to limit the security impact of some attachments diff --git a/lib/pleroma/web/plugs/uploaded_media.ex b/lib/pleroma/web/plugs/uploaded_media.ex index 9dd5eb2398..0d71481635 100644 --- a/lib/pleroma/web/plugs/uploaded_media.ex +++ b/lib/pleroma/web/plugs/uploaded_media.ex @@ -55,6 +55,7 @@ def call(%{request_path: <<"/", @path, "/", file::binary>>} = conn, opts) do {:ok, get_method} <- uploader.get_file(file), false <- media_is_banned(conn, get_method) do get_media(conn, get_method, proxy_remote, opts) + |> scrub_mime() else {:valid_host, false} -> redirect_url = @@ -131,4 +132,26 @@ defp get_media(conn, unknown, _, _) do |> send_resp(:internal_server_error, dgettext("errors", "Internal Error")) |> halt() end + + defp scrub_mime(%Plug.Conn{resp_headers: headers} = conn) do + [{_, mimetype}] = Enum.filter(headers, fn {x, _y} -> match?("content-type", x) end) + + [_type, subtype] = String.split(mimetype, "/") + + cond do + String.contains?(subtype, ["javascript", "ecmascript", "jscript"]) -> + force_plaintext(conn) + + String.contains?(mimetype, ["text/html", "text/xml", "application/xml"]) -> + force_plaintext(conn) + + true -> + conn + end + end + + defp force_plaintext(conn) do + conn + |> merge_resp_headers([{"content-type", "text/plain"}]) + end end diff --git a/test/fixtures/snow.js b/test/fixtures/snow.js new file mode 100644 index 0000000000..ff53af709c Binary files /dev/null and b/test/fixtures/snow.js differ diff --git a/test/pleroma/web/plugs/uploaded_media_plug_test.exs b/test/pleroma/web/plugs/uploaded_media_plug_test.exs index dbf8ca5ec4..7033181b02 100644 --- a/test/pleroma/web/plugs/uploaded_media_plug_test.exs +++ b/test/pleroma/web/plugs/uploaded_media_plug_test.exs @@ -66,4 +66,33 @@ test "denies access to media if wrong Host", %{ assert redirected_to(conn, 302) == expected_url end + + test "Filters out dangerous content types" do + context = %{module: __MODULE__, case: __MODULE__} + + test_files = [ + "test/fixtures/lain.xml", + "test/fixtures/nypd-facial-recognition-children-teenagers.html", + "test/fixtures/snow.js" + ] + + Enum.each(test_files, fn t -> + Pleroma.DataCase.ensure_local_uploader(context) + filename = String.split(t, "/") |> List.last() + + upload = %Plug.Upload{ + path: Path.absname(t), + filename: filename + } + + {:ok, %{"url" => [%{"href" => attachment_url}]}} = Upload.store(upload) + + conn = get(build_conn(), attachment_url) + + assert Enum.any?( + conn.resp_headers, + &(&1 == {"content-type", "text/plain"}) + ) + end) + end end