From 96fe080e6eea08aeedf0cd982e997168d3d10d48 Mon Sep 17 00:00:00 2001 From: Oneric Date: Wed, 30 Oct 2024 23:24:05 +0100 Subject: [PATCH] Convert all raw :zip usage to SafeZip Notably at least two instances were not properly guarded from path traversal attack before and are only now fixed by using SafeZip: - frontend installation did never check for malicious paths. But given a malicious froontend could already, e.g. steal all user tokens even without this, in the real world admins should only use frontends from trusted sources and the practical implications are minimal - the emoji pack update/upload API taking a ZIP file did not protect against path traversal. While atm only admins can use these emoji endpoints, emoji packs are typically considered "harmless" and used without prior verification from various sources. Thus this appears more concerning. --- lib/mix/tasks/pleroma/emoji.ex | 15 ++----- lib/pleroma/emoji/pack.ex | 75 +++++++++++++++------------------- lib/pleroma/frontend.ex | 23 +++-------- lib/pleroma/user/backup.ex | 7 ++-- 4 files changed, 43 insertions(+), 77 deletions(-) diff --git a/lib/mix/tasks/pleroma/emoji.ex b/lib/mix/tasks/pleroma/emoji.ex index aa8131254..2b13b3257 100644 --- a/lib/mix/tasks/pleroma/emoji.ex +++ b/lib/mix/tasks/pleroma/emoji.ex @@ -93,6 +93,7 @@ def run(["get-packs" | args]) do ) files = fetch_and_decode!(files_loc) + files_to_unzip = for({_, f} <- files, do: f) shell_info(IO.ANSI.format(["Unpacking ", :bright, pack_name])) @@ -103,17 +104,7 @@ def run(["get-packs" | args]) do pack_name ]) - files_to_unzip = - Enum.map( - files, - fn {_, f} -> to_charlist(f) end - ) - - {:ok, _} = - :zip.unzip(binary_archive, - cwd: to_charlist(pack_path), - file_list: files_to_unzip - ) + {:ok, _} = Pleroma.SafeZip.unzip_data(binary_archive, pack_path, files_to_unzip) shell_info(IO.ANSI.format(["Writing pack.json for ", :bright, pack_name])) @@ -202,7 +193,7 @@ def run(["gen-pack" | args]) do tmp_pack_dir = Path.join(System.tmp_dir!(), "emoji-pack-#{name}") - {:ok, _} = :zip.unzip(binary_archive, cwd: String.to_charlist(tmp_pack_dir)) + {:ok, _} = Pleroma.SafeZip.unzip_data(binary_archive, tmp_pack_dir) emoji_map = Pleroma.Emoji.Loader.make_shortcode_to_file_map(tmp_pack_dir, exts) diff --git a/lib/pleroma/emoji/pack.ex b/lib/pleroma/emoji/pack.ex index e95320a07..fdd31d165 100644 --- a/lib/pleroma/emoji/pack.ex +++ b/lib/pleroma/emoji/pack.ex @@ -25,6 +25,7 @@ defmodule Pleroma.Emoji.Pack do alias Pleroma.Emoji alias Pleroma.Emoji.Pack alias Pleroma.Utils + alias Pleroma.SafeZip # 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 @@ -95,22 +96,20 @@ def delete(name) do end end - @spec unpack_zip_emojies(list(tuple())) :: list(map()) - defp unpack_zip_emojies(zip_files) do - Enum.reduce(zip_files, [], fn - {_, path, s, _, _, _}, acc when elem(s, 2) == :regular -> - with( - filename <- Path.basename(path), - shortcode <- Path.basename(filename, Path.extname(filename)), - false <- Emoji.exist?(shortcode) - ) do - [%{path: path, filename: path, shortcode: shortcode} | acc] - else - _ -> acc - end - - _, acc -> - acc + @spec map_zip_emojies(list(String.t())) :: list(map()) + defp map_zip_emojies(zip_files) do + Enum.reduce(zip_files, [], fn path, acc -> + with( + filename <- Path.basename(path), + shortcode <- Path.basename(filename, Path.extname(filename)), + # note: this only checks the shortcode, if an emoji already exists on the same path, but + # with a different shortcode, the existing one will be degraded to an alias of the new + false <- Emoji.exist?(shortcode) + ) do + [%{path: path, filename: path, shortcode: shortcode} | acc] + else + _ -> acc + end end) end @@ -118,15 +117,12 @@ defp unpack_zip_emojies(zip_files) do {:ok, t()} | {:error, File.posix() | atom()} def add_file(%Pack{} = pack, _, _, %Plug.Upload{content_type: "application/zip"} = file) do - with {:ok, zip_files} <- :zip.table(to_charlist(file.path)), - [_ | _] = emojies <- unpack_zip_emojies(zip_files), + with {:ok, zip_files} <- SafeZip.list_dir_file(file.path), + [_ | _] = emojies <- map_zip_emojies(zip_files), {:ok, tmp_dir} <- Utils.tmp_dir("emoji") do try do {:ok, _emoji_files} = - :zip.unzip( - to_charlist(file.path), - [{:file_list, Enum.map(emojies, & &1[:path])}, {:cwd, to_charlist(tmp_dir)}] - ) + SafeZip.unzip_file(file.path, tmp_dir, Enum.map(emojies, & &1[:path])) {_, updated_pack} = Enum.map_reduce(emojies, pack, fn item, emoji_pack -> @@ -446,16 +442,9 @@ defp downloadable?(pack) do end defp create_archive_and_cache(pack, hash) do - 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)]) + pack_file_list = Enum.into(pack.files, [], fn {_, f} -> f end) + files = ["pack.json" | pack_file_list] + {:ok, {_, result}} = SafeZip.zip("#{pack.name}.zip", files, pack.path, true) ttl_per_file = Pleroma.Config.get!([:emoji, :shared_pack_cache_seconds_per_file]) overall_ttl = :timer.seconds(ttl_per_file * Enum.count(files)) @@ -626,11 +615,10 @@ defp copy_as(remote_pack, local_name) do defp unzip(archive, pack_info, remote_pack, local_pack) do with :ok <- File.mkdir_p!(local_pack.path) do - files = Enum.map(remote_pack["files"], fn {_, path} -> to_charlist(path) end) + files = Enum.map(remote_pack["files"], fn {_, path} -> path end) # Fallback cannot contain a pack.json file - files = if pack_info[:fallback], do: files, else: [~c"pack.json" | files] - - :zip.unzip(archive, cwd: to_charlist(local_pack.path), file_list: files) + files = if pack_info[:fallback], do: files, else: ["pack.json" | files] + SafeZip.unzip_data(archive, local_pack.path, files) end end @@ -693,13 +681,14 @@ defp update_sha_and_save_metadata(pack, data) do end defp validate_has_all_files(pack, zip) do - with {:ok, f_list} <- :zip.unzip(zip, [:memory]) do - # Check if all files from the pack.json are in the archive - pack.files - |> Enum.all?(fn {_, from_manifest} -> - List.keyfind(f_list, to_charlist(from_manifest), 0) + # Check if all files from the pack.json are in the archive + eset = + Enum.reduce(pack.files, MapSet.new(), fn + {_, file}, s -> MapSet.put(s, to_charlist(file)) end) - |> if(do: :ok, else: {:error, :incomplete}) - end + + if SafeZip.contains_all_data?(zip, eset), + do: :ok, + else: {:error, :incomplete} end end diff --git a/lib/pleroma/frontend.ex b/lib/pleroma/frontend.ex index a309d8467..3ce7910cf 100644 --- a/lib/pleroma/frontend.ex +++ b/lib/pleroma/frontend.ex @@ -70,25 +70,12 @@ defp download_or_unzip(_frontend_info, temp_dir, file) do end def unzip(zip, dest) do - with {:ok, unzipped} <- :zip.unzip(zip, [:memory]) do - File.rm_rf!(dest) - File.mkdir_p!(dest) + File.rm_rf!(dest) + File.mkdir_p!(dest) - Enum.each(unzipped, fn {filename, data} -> - path = filename - - new_file_path = Path.join(dest, path) - - new_file_path - |> Path.dirname() - |> File.rm() - - new_file_path - |> Path.dirname() - |> File.mkdir_p!() - - File.write!(new_file_path, data) - end) + case Pleroma.SafeZip.unzip_data(zip, dest) do + {:ok, _} -> :ok + error -> error end end diff --git a/lib/pleroma/user/backup.ex b/lib/pleroma/user/backup.ex index de967abe3..21c2948a0 100644 --- a/lib/pleroma/user/backup.ex +++ b/lib/pleroma/user/backup.ex @@ -119,7 +119,7 @@ def process(%__MODULE__{} = backup) do end end - @files [~c"actor.json", ~c"outbox.json", ~c"likes.json", ~c"bookmarks.json"] + @files ["actor.json", "outbox.json", "likes.json", "bookmarks.json"] def export(%__MODULE__{} = backup) do backup = Repo.preload(backup, :user) name = String.trim_trailing(backup.file_name, ".zip") @@ -130,10 +130,9 @@ def export(%__MODULE__{} = backup) do :ok <- statuses(dir, backup.user), :ok <- likes(dir, backup.user), :ok <- bookmarks(dir, backup.user), - {:ok, zip_path} <- - :zip.create(String.to_charlist(dir <> ".zip"), @files, cwd: String.to_charlist(dir)), + {:ok, zip_path} <- Pleroma.SafeZip.zip(dir <> ".zip", @files, dir), {:ok, _} <- File.rm_rf(dir) do - {:ok, to_string(zip_path)} + {:ok, zip_path} end end