From e4925f813afda5883fd12a48b99b2b12f83678d9 Mon Sep 17 00:00:00 2001 From: tusooa Date: Thu, 29 Dec 2022 14:33:46 -0500 Subject: [PATCH] Sanitize filenames when uploading --- lib/pleroma/web/activity_pub/activity_pub.ex | 11 +++++++- .../web/activity_pub/activity_pub_test.exs | 8 ++++++ .../controllers/media_controller_test.exs | 17 +++++++++++ .../mastodon_api/update_credentials_test.exs | 28 +++++++++++++++++++ 4 files changed, 63 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index b9206b4da..1ab2db94a 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1453,13 +1453,22 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do @spec upload(Upload.source(), keyword()) :: {:ok, Object.t()} | {:error, any()} def upload(file, opts \\ []) do - with {:ok, data} <- Upload.store(file, opts) do + with {:ok, data} <- Upload.store(sanitize_upload_file(file), opts) do obj_data = Maps.put_if_present(data, "actor", opts[:actor]) Repo.insert(%Object{data: obj_data}) end end + defp sanitize_upload_file(%Plug.Upload{filename: filename} = upload) when is_binary(filename) do + %Plug.Upload{ + upload + | filename: Path.basename(filename) + } + end + + defp sanitize_upload_file(upload), do: upload + @spec get_actor_url(any()) :: binary() | nil defp get_actor_url(url) when is_binary(url), do: url defp get_actor_url(%{"href" => href}) when is_binary(href), do: href diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs index fc6fc039d..a6f8b6152 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_test.exs @@ -1342,6 +1342,14 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do %{test_file: test_file} end + test "strips / from filename", %{test_file: file} do + file = %Plug.Upload{file | filename: "../../../../../nested/bad.jpg"} + {:ok, %Object{} = object} = ActivityPub.upload(file) + [%{"href" => href}] = object.data["url"] + assert Regex.match?(~r"/bad.jpg$", href) + refute Regex.match?(~r"/nested/", href) + end + test "sets a description if given", %{test_file: file} do {:ok, %Object{} = object} = ActivityPub.upload(file, description: "a cool file") assert object.data["name"] == "a cool file" diff --git a/test/pleroma/web/mastodon_api/controllers/media_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/media_controller_test.exs index 79d52bb2f..750296230 100644 --- a/test/pleroma/web/mastodon_api/controllers/media_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/media_controller_test.exs @@ -122,6 +122,23 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do assert :ok == File.rm(Path.absname("test/tmp/large_binary.data")) end + + test "Do not allow nested filename", %{conn: conn, image: image} do + image = %Plug.Upload{ + image + | filename: "../../../../../nested/file.jpg" + } + + desc = "Description of the image" + + media = + conn + |> put_req_header("content-type", "multipart/form-data") + |> post("/api/v1/media", %{"file" => image, "description" => desc}) + |> json_response_and_validate_schema(:ok) + + refute Regex.match?(~r"/nested/", media["url"]) + end end describe "Update media description" do diff --git a/test/pleroma/web/mastodon_api/update_credentials_test.exs b/test/pleroma/web/mastodon_api/update_credentials_test.exs index d5fac7e25..57fa0f047 100644 --- a/test/pleroma/web/mastodon_api/update_credentials_test.exs +++ b/test/pleroma/web/mastodon_api/update_credentials_test.exs @@ -383,6 +383,34 @@ defmodule Pleroma.Web.MastodonAPI.UpdateCredentialsTest do assert :ok == File.rm(Path.absname("test/tmp/large_binary.data")) end + test "Strip / from upload files", %{user: user, conn: conn} do + new_image = %Plug.Upload{ + content_type: "image/jpeg", + path: Path.absname("test/fixtures/image.jpg"), + filename: "../../../../nested/an_image.jpg" + } + + assert user.avatar == %{} + + res = + patch(conn, "/api/v1/accounts/update_credentials", %{ + "avatar" => new_image, + "header" => new_image, + "pleroma_background_image" => new_image + }) + + assert user_response = json_response_and_validate_schema(res, 200) + assert user_response["avatar"] + assert user_response["header"] + assert user_response["pleroma"]["background_image"] + refute Regex.match?(~r"/nested/", user_response["avatar"]) + refute Regex.match?(~r"/nested/", user_response["header"]) + refute Regex.match?(~r"/nested/", user_response["pleroma"]["background_image"]) + + user = User.get_by_id(user.id) + refute user.avatar == %{} + end + test "requires 'write:accounts' permission" do token1 = insert(:oauth_token, scopes: ["read"]) token2 = insert(:oauth_token, scopes: ["write", "follow"])