Merge pull request 'Apply security patch from pleroma to prevent nested file names being uploaded to the server.' (#507) from foxing/akkoma:foxing-patch-2 into develop

Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/507
This commit is contained in:
floatingghost 2023-03-13 00:29:51 +00:00
commit 377d1483b6
4 changed files with 63 additions and 1 deletions

View file

@ -1502,13 +1502,22 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
@spec upload(Upload.source(), keyword()) :: {:ok, Object.t()} | {:error, any()} @spec upload(Upload.source(), keyword()) :: {:ok, Object.t()} | {:error, any()}
def upload(file, opts \\ []) do 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]) obj_data = Maps.put_if_present(data, "actor", opts[:actor])
Repo.insert(%Object{data: obj_data}) Repo.insert(%Object{data: obj_data})
end end
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 @spec get_actor_url(any()) :: binary() | nil
defp get_actor_url(url) when is_binary(url), do: url defp get_actor_url(url) when is_binary(url), do: url
defp get_actor_url(%{"href" => href}) when is_binary(href), do: href defp get_actor_url(%{"href" => href}) when is_binary(href), do: href

View file

@ -1303,6 +1303,14 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
%{test_file: test_file} %{test_file: test_file}
end 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 test "sets a description if given", %{test_file: file} do
{:ok, %Object{} = object} = ActivityPub.upload(file, description: "a cool file") {:ok, %Object{} = object} = ActivityPub.upload(file, description: "a cool file")
assert object.data["name"] == "a cool file" assert object.data["name"] == "a cool file"

View file

@ -124,6 +124,23 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do
assert :ok == File.rm(Path.absname("test/tmp/large_binary.data")) assert :ok == File.rm(Path.absname("test/tmp/large_binary.data"))
end 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 end
describe "Update media description" do describe "Update media description" do

View file

@ -396,6 +396,34 @@ defmodule Pleroma.Web.MastodonAPI.UpdateCredentialsTest do
assert :ok == File.rm(Path.absname("test/tmp/large_binary.data")) assert :ok == File.rm(Path.absname("test/tmp/large_binary.data"))
end 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 test "requires 'write:accounts' permission" do
token1 = insert(:oauth_token, scopes: ["read"]) token1 = insert(:oauth_token, scopes: ["read"])
token2 = insert(:oauth_token, scopes: ["write", "follow"]) token2 = insert(:oauth_token, scopes: ["write", "follow"])