Proactively harden emoji pack against path traversal

No new path traversal attacks are known. But given the many entrypoints
and code flow complexity inside pack.ex, it unfortunately seems
possible a future refactor or addition might reintroduce one.
Furthermore, some old packs might still contain traversing path entries
which could trigger undesireable actions on rename or delete.

To ensure this can never happen, assert safety during path construction.

Path.safe_relative was introduced in Elixir 1.14, but
fortunately, we already require at least 1.14 anyway.
This commit is contained in:
Oneric 2024-03-10 07:15:26 +01:00
parent d6d838cbe8
commit ddd79ff22d
2 changed files with 46 additions and 15 deletions

View file

@ -26,10 +26,32 @@ defmodule Pleroma.Emoji.Pack do
alias Pleroma.Emoji.Pack alias Pleroma.Emoji.Pack
alias Pleroma.Utils 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} @spec create(String.t()) :: {:ok, t()} | {:error, File.posix()} | {:error, :empty_values}
def create(name) do def create(name) do
with :ok <- validate_not_empty([name]), 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 :ok <- File.mkdir(dir) do
save_pack(%__MODULE__{ save_pack(%__MODULE__{
path: dir, path: dir,
@ -68,7 +90,7 @@ def show(opts) do
{:ok, [binary()]} | {:error, File.posix(), binary()} | {:error, :empty_values} {:ok, [binary()]} | {:error, File.posix(), binary()} | {:error, :empty_values}
def delete(name) do def delete(name) do
with :ok <- validate_not_empty([name]), 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) File.rm_rf(pack_path)
end end
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 -> Enum.map_reduce(emojies, pack, fn item, emoji_pack ->
emoji_file = %Plug.Upload{ emoji_file = %Plug.Upload{
filename: item[:filename], filename: item[:filename],
path: Path.join(tmp_dir, item[:path]) path: path_join_safe(tmp_dir, item[:path])
} }
{:ok, updated_pack} = {:ok, updated_pack} =
@ -200,6 +222,7 @@ def import_from_filesystem do
{:ok, results} <- File.ls(emoji_path) do {:ok, results} <- File.ls(emoji_path) do
names = names =
results results
# items come from File.ls, thus safe
|> Enum.map(&Path.join(emoji_path, &1)) |> Enum.map(&Path.join(emoji_path, &1))
|> Enum.reject(fn path -> |> Enum.reject(fn path ->
File.dir?(path) and File.exists?(Path.join(path, "pack.json")) 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()} @spec load_pack(String.t()) :: {:ok, t()} | {:error, :file.posix()}
def load_pack(name) do def load_pack(name) do
name = Path.basename(name) pack_dir = path_join_name_safe(emoji_path(), name)
pack_file = Path.join([emoji_path(), name, "pack.json"]) pack_file = Path.join(pack_dir, "pack.json")
with {:ok, _} <- File.stat(pack_file), with {:ok, _} <- File.stat(pack_file),
{:ok, pack_data} <- File.read(pack_file) do {:ok, pack_data} <- File.read(pack_file) do
@ -423,7 +446,13 @@ defp downloadable?(pack) do
end end
defp create_archive_and_cache(pack, hash) do 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}} = {:ok, {_, result}} =
:zip.zip(~c"#{pack.name}.zip", files, [:memory, cwd: to_charlist(pack.path)]) :zip.zip(~c"#{pack.name}.zip", files, [:memory, cwd: to_charlist(pack.path)])
@ -485,7 +514,7 @@ defp validate_not_empty(list) do
end end
defp save_file(%Plug.Upload{path: upload_path}, pack, filename) do 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) create_subdirs(file_path)
with {:ok, _} <- File.copy(upload_path, file_path) do 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 end
defp save_file(file_data, pack, filename) when is_binary(file_data) do 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) create_subdirs(file_path)
File.write(file_path, file_data, [:binary]) File.write(file_path, file_data, [:binary])
end end
@ -510,8 +539,8 @@ defp delete_emoji(pack, shortcode) do
end end
defp rename_file(pack, filename, new_filename) do defp rename_file(pack, filename, new_filename) do
old_path = Path.join(pack.path, filename) old_path = path_join_safe(pack.path, filename)
new_path = Path.join(pack.path, new_filename) new_path = path_join_safe(pack.path, new_filename)
create_subdirs(new_path) create_subdirs(new_path)
with :ok <- File.rename(old_path, new_path) do 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 defp remove_file(pack, shortcode) do
with {:ok, filename} <- get_filename(pack, shortcode), 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 :ok <- File.rm(emoji) do
remove_dir_if_empty(emoji, filename) remove_dir_if_empty(emoji, filename)
end end
@ -547,7 +576,7 @@ defp remove_dir_if_empty(emoji, filename) do
defp get_filename(pack, shortcode) do defp get_filename(pack, shortcode) do
with %{^shortcode => filename} when is_binary(filename) <- pack.files, 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, _} <- File.stat(file_path) do
{:ok, filename} {:ok, filename}
else else
@ -585,7 +614,7 @@ defp validate_downloadable(pack) do
end end
defp copy_as(remote_pack, local_name) do 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__{ %__MODULE__{
name: local_name, name: local_name,

View file

@ -93,7 +93,9 @@ test "add emoji file", %{pack: pack} do
assert updated_pack.files_count == 1 assert updated_pack.files_count == 1
end end
test "load_pack/1 ignores path traversal in a forged pack name", %{pack: pack} do test "load_pack/1 panics on path traversal in a forged pack name" do
assert {:ok, ^pack} = Pack.load_pack("../../../../../dump_pack") assert_raise(RuntimeError, "Invalid or malicious pack name: ../../../../../dump_pack", fn ->
Pack.load_pack("../../../../../dump_pack")
end)
end end
end end