Merge branch 'develop' into foxing-patch-1

Reviewed-on: AkkomaGang/akkoma#507

lib/pleroma/user.ex

@@ -366,21 +366,21 @@ defmodule Pleroma.User do
   def invisible?(_), do: false
 
   def avatar_url(user, options \\ []) do
-    case user.avatar do
-      %{"url" => [%{"href" => href} | _]} ->
-        href
-
-      _ ->
-        unless options[:no_default] do
-          Config.get([:assets, :default_user_avatar], "#{Endpoint.url()}/images/avi.png")
-        end
-    end
+    default = Config.get([:assets, :default_user_avatar], "#{Endpoint.url()}/images/avi.png")
+    do_optional_url(user.avatar, default, options)
   end
 
   def banner_url(user, options \\ []) do
-    case user.banner do
-      %{"url" => [%{"href" => href} | _]} -> href
-      _ -> !options[:no_default] && "#{Endpoint.url()}/images/banner.png"
+    do_optional_url(user.banner, "#{Endpoint.url()}/images/banner.png", options)
+  end
+
+  defp do_optional_url(field, default, options \\ []) do
+    case field do
+      %{"url" => [%{"href" => href} | _]} when is_binary(href) ->
+        href
+
+      _ ->
+        unless options[:no_default], do: default
     end
   end
 

lib/pleroma/web/activity_pub/activity_pub.ex

@@ -1502,13 +1502,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

test/pleroma/user_test.exs

@@ -2509,6 +2509,16 @@ defmodule Pleroma.UserTest do
     assert User.avatar_url(user, no_default: true) == nil
   end
 
+  test "avatar object with nil in href" do
+    user = insert(:user, avatar: %{"url" => [%{"href" => nil}]})
+    assert User.avatar_url(user) != nil
+  end
+
+  test "banner object with nil in href" do
+    user = insert(:user, banner: %{"url" => [%{"href" => nil}]})
+    assert User.banner_url(user) != nil
+  end
+
   test "get_host/1" do
     user = insert(:user, ap_id: "https://lain.com/users/lain", nickname: "lain")
     assert User.get_host(user) == "lain.com"

test/pleroma/web/activity_pub/activity_pub_test.exs

@@ -1303,6 +1303,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"

test/pleroma/web/mastodon_api/controllers/media_controller_test.exs

@@ -124,6 +124,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

test/pleroma/web/mastodon_api/update_credentials_test.exs

@@ -396,6 +396,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"])