pleroma

commit: e4925f813afda5883fd12a48b99b2b12f83678d9
parent 5d34fe1868b152a607b1734b5bbc3e7e43c70f28
Author: tusooa <tusooa@kazv.moe>
Date:   Thu, 29 Dec 2022 14:33:46 -0500

Sanitize filenames when uploading

Diffstat:
M
lib/pleroma/web/activity_pub/activity_pub.ex
11
++++++++++-
M
test/pleroma/web/activity_pub/activity_pub_test.exs
8
++++++++
M
test/pleroma/web/mastodon_api/controllers/media_controller_test.exs
17
+++++++++++++++++
M
test/pleroma/web/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
@@ -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
@@ -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
@@ -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
@@ -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"])