diff --git a/lib/pleroma/emoji/pack.ex b/lib/pleroma/emoji/pack.ex index f007cde65..142208854 100644 --- a/lib/pleroma/emoji/pack.ex +++ b/lib/pleroma/emoji/pack.ex @@ -26,10 +26,32 @@ defmodule Pleroma.Emoji.Pack do alias Pleroma.Emoji.Pack alias Pleroma.Utils + # Invalid/Malicious names are supposed to be filtered out before path joining, + # but there are many entrypoints to affected functions so as the code changes + # we might accidentally let an unsanitised name slip through. + # To make sure, use the below which crash the process otherwise. + + # ALWAYS use this when constructing paths from external name! + # (name meaning it must be only a single path component) + defp path_join_name_safe(dir, name) do + if to_string(name) != Path.basename(name) or name in ["..", ".", ""] do + raise "Invalid or malicious pack name: #{name}" + else + Path.join(dir, name) + end + end + + # ALWAYS use this to join external paths + # (which are allowed to have several components) + defp path_join_safe(dir, path) do + {:ok, safe_path} = Path.safe_relative(path) + Path.join(dir, safe_path) + end + @spec create(String.t()) :: {:ok, t()} | {:error, File.posix()} | {:error, :empty_values} def create(name) do with :ok <- validate_not_empty([name]), - dir <- Path.join(emoji_path(), name), + dir <- path_join_name_safe(emoji_path(), name), :ok <- File.mkdir(dir) do save_pack(%__MODULE__{ path: dir, @@ -68,7 +90,7 @@ def show(opts) do {:ok, [binary()]} | {:error, File.posix(), binary()} | {:error, :empty_values} def delete(name) do with :ok <- validate_not_empty([name]), - pack_path <- Path.join(emoji_path(), name) do + pack_path <- path_join_name_safe(emoji_path(), name) do File.rm_rf(pack_path) end end @@ -110,7 +132,7 @@ def add_file(%Pack{} = pack, _, _, %Plug.Upload{content_type: "application/zip"} Enum.map_reduce(emojies, pack, fn item, emoji_pack -> emoji_file = %Plug.Upload{ filename: item[:filename], - path: Path.join(tmp_dir, item[:path]) + path: path_join_safe(tmp_dir, item[:path]) } {:ok, updated_pack} = @@ -200,6 +222,7 @@ def import_from_filesystem do {:ok, results} <- File.ls(emoji_path) do names = results + # items come from File.ls, thus safe |> Enum.map(&Path.join(emoji_path, &1)) |> Enum.reject(fn path -> File.dir?(path) and File.exists?(Path.join(path, "pack.json")) @@ -298,8 +321,8 @@ def update_metadata(name, data) do @spec load_pack(String.t()) :: {:ok, t()} | {:error, :file.posix()} def load_pack(name) do - name = Path.basename(name) - pack_file = Path.join([emoji_path(), name, "pack.json"]) + pack_dir = path_join_name_safe(emoji_path(), name) + pack_file = Path.join(pack_dir, "pack.json") with {:ok, _} <- File.stat(pack_file), {:ok, pack_data} <- File.read(pack_file) do @@ -423,7 +446,13 @@ defp downloadable?(pack) do end defp create_archive_and_cache(pack, hash) do - files = [~c"pack.json" | Enum.map(pack.files, fn {_, file} -> to_charlist(file) end)] + files = [ + ~c"pack.json" + | Enum.map(pack.files, fn {_, file} -> + {:ok, file} = Path.safe_relative(file) + to_charlist(file) + end) + ] {:ok, {_, result}} = :zip.zip(~c"#{pack.name}.zip", files, [:memory, cwd: to_charlist(pack.path)]) @@ -485,7 +514,7 @@ defp validate_not_empty(list) do end defp save_file(%Plug.Upload{path: upload_path}, pack, filename) do - file_path = Path.join(pack.path, filename) + file_path = path_join_safe(pack.path, filename) create_subdirs(file_path) with {:ok, _} <- File.copy(upload_path, file_path) do @@ -494,7 +523,7 @@ defp save_file(%Plug.Upload{path: upload_path}, pack, filename) do end defp save_file(file_data, pack, filename) when is_binary(file_data) do - file_path = Path.join(pack.path, filename) + file_path = path_join_safe(pack.path, filename) create_subdirs(file_path) File.write(file_path, file_data, [:binary]) end @@ -510,8 +539,8 @@ defp delete_emoji(pack, shortcode) do end defp rename_file(pack, filename, new_filename) do - old_path = Path.join(pack.path, filename) - new_path = Path.join(pack.path, new_filename) + old_path = path_join_safe(pack.path, filename) + new_path = path_join_safe(pack.path, new_filename) create_subdirs(new_path) with :ok <- File.rename(old_path, new_path) do @@ -529,7 +558,7 @@ defp create_subdirs(file_path) do defp remove_file(pack, shortcode) do with {:ok, filename} <- get_filename(pack, shortcode), - emoji <- Path.join(pack.path, filename), + emoji <- path_join_safe(pack.path, filename), :ok <- File.rm(emoji) do remove_dir_if_empty(emoji, filename) end @@ -547,7 +576,7 @@ defp remove_dir_if_empty(emoji, filename) do defp get_filename(pack, shortcode) do with %{^shortcode => filename} when is_binary(filename) <- pack.files, - file_path <- Path.join(pack.path, filename), + file_path <- path_join_safe(pack.path, filename), {:ok, _} <- File.stat(file_path) do {:ok, filename} else @@ -585,7 +614,7 @@ defp validate_downloadable(pack) do end defp copy_as(remote_pack, local_name) do - path = Path.join(emoji_path(), local_name) + path = path_join_name_safe(emoji_path(), local_name) %__MODULE__{ name: local_name, diff --git a/test/pleroma/emoji/pack_test.exs b/test/pleroma/emoji/pack_test.exs index 4d769789d..f5d2e2eef 100644 --- a/test/pleroma/emoji/pack_test.exs +++ b/test/pleroma/emoji/pack_test.exs @@ -93,7 +93,9 @@ test "add emoji file", %{pack: pack} do assert updated_pack.files_count == 1 end - test "load_pack/1 ignores path traversal in a forged pack name", %{pack: pack} do - assert {:ok, ^pack} = Pack.load_pack("../../../../../dump_pack") + test "load_pack/1 panics on path traversal in a forged pack name" do + assert_raise(RuntimeError, "Invalid or malicious pack name: ../../../../../dump_pack", fn -> + Pack.load_pack("../../../../../dump_pack") + end) end end