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.
This commit is contained in:
Oneric 2024-10-30 23:24:05 +01:00
parent 7151ef4718
commit 96fe080e6e
4 changed files with 43 additions and 77 deletions

View file

@ -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)

View file

@ -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

View file

@ -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

View file

@ -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