Restrict media usage to owners

In Mastodon media can only be used by owners and only be associated with
a single post. We currently allow media to be associated with several
posts and until now did not limit their usage in posts to media owners.
However, media update and GET lookup was already limited to owners.
(In accordance with allowing media reuse, we also still allow GET
lookups of media already used in a post unlike Mastodon)

Allowing reuse isn’t problematic per se, but allowing use by non-owners
can be problematic if media ids of private-scoped posts can be guessed
since creating a new post with this media id will reveal the uploaded
file content and alt text.
Given media ids are currently just part of a sequentieal series shared
with some other objects, guessing media ids is with some persistence
indeed feasible.

E.g. sampline some public media ids from a real-world
instance with 112 total and 61 monthly-active users:

  17.465.096  at  t0
  17.472.673  at  t1 = t0 + 4h
  17.473.248  at  t2 = t1 + 20min

This gives about 30 new ids per minute of which most won't be
local media but remote and local posts, poll answers etc.
Assuming the default ratelimit of 15 post actions per 10s, scraping all
media for the 4h interval takes about 84 minutes and scraping the 20min
range mere 6.3 minutes. (Until the preceding commit, post updates were
not rate limited at all, allowing even faster scraping.)
If an attacker can infer (e.g. via reply to a follower-only post not
accessbile to the attacker) some sensitive information was uploaded
during a specific time interval and has some pointers regarding the
nature of the information, identifying the specific upload out of all
scraped media for this timerange is not impossible.

Thus restrict media usage to owners.

Checking ownership just in ActivitDraft would already be sufficient,
since when a scheduled status actually gets posted it goes through
ActivityDraft again, but would erroneously return a success status
when scheduling an illegal post.

Independently discovered and fixed by mint in Pleroma
1afde067b1
This commit is contained in:
Oneric 2024-04-24 17:46:18 +02:00
parent 6ef6b2a289
commit 0c2b33458d
7 changed files with 125 additions and 33 deletions

View file

@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
## Unreleased ## Unreleased
## Fixed
- Issue allowing non-owners to use media objects in posts
## 2024.04 ## 2024.04
## Added ## Added

View file

@ -40,19 +40,29 @@ defmodule Pleroma.ScheduledActivity do
%{changes: %{params: %{"media_ids" => media_ids} = params}} = changeset %{changes: %{params: %{"media_ids" => media_ids} = params}} = changeset
) )
when is_list(media_ids) do when is_list(media_ids) do
media_attachments = Utils.attachments_from_ids(%{media_ids: media_ids}) user = User.get_by_id(changeset.data.user_id)
params = case Utils.attachments_from_ids(user, %{media_ids: media_ids}) do
params media_attachments when is_list(media_attachments) ->
|> Map.put("media_attachments", media_attachments) params =
|> Map.put("media_ids", media_ids) params
|> Map.put("media_attachments", media_attachments)
|> Map.put("media_ids", media_ids)
put_change(changeset, :params, params) put_change(changeset, :params, params)
{:error, _} = e ->
e
e ->
{:error, e}
end
end end
defp with_media_attachments(changeset), do: changeset defp with_media_attachments(changeset), do: changeset
defp update_changeset(%ScheduledActivity{} = scheduled_activity, attrs) do defp update_changeset(%ScheduledActivity{} = scheduled_activity, attrs) do
# note: should this ever allow swapping media attachments, make sure ownership is checked
scheduled_activity scheduled_activity
|> cast(attrs, [:scheduled_at]) |> cast(attrs, [:scheduled_at])
|> validate_required([:scheduled_at]) |> validate_required([:scheduled_at])
@ -115,13 +125,22 @@ defmodule Pleroma.ScheduledActivity do
@doc """ @doc """
Creates ScheduledActivity and add to queue to perform at scheduled_at date Creates ScheduledActivity and add to queue to perform at scheduled_at date
""" """
@spec create(User.t(), map()) :: {:ok, ScheduledActivity.t()} | {:error, Ecto.Changeset.t()} @spec create(User.t(), map()) :: {:ok, ScheduledActivity.t()} | {:error, any()}
def create(%User{} = user, attrs) do def create(%User{} = user, attrs) do
Multi.new() case new(user, attrs) do
|> Multi.insert(:scheduled_activity, new(user, attrs)) %Ecto.Changeset{} = sched_data ->
|> maybe_add_jobs(Config.get([ScheduledActivity, :enabled])) Multi.new()
|> Repo.transaction() |> Multi.insert(:scheduled_activity, sched_data)
|> transaction_response |> maybe_add_jobs(Config.get([ScheduledActivity, :enabled]))
|> Repo.transaction()
|> transaction_response
{:error, _} = e ->
e
e ->
{:error, e}
end
end end
defp maybe_add_jobs(multi, true) do defp maybe_add_jobs(multi, true) do

View file

@ -92,9 +92,14 @@ defmodule Pleroma.Web.CommonAPI.ActivityDraft do
end end
end end
defp attachments(%{params: params} = draft) do defp attachments(%{params: params, user: user} = draft) do
attachments = Utils.attachments_from_ids(params) case Utils.attachments_from_ids(user, params) do
%__MODULE__{draft | attachments: attachments} attachments when is_list(attachments) ->
%__MODULE__{draft | attachments: attachments}
{:error, reason} ->
add_error(draft, reason)
end
end end
defp in_reply_to(%{params: %{in_reply_to_status_id: ""}} = draft), do: draft defp in_reply_to(%{params: %{in_reply_to_status_id: ""}} = draft), do: draft

View file

@ -22,24 +22,24 @@ defmodule Pleroma.Web.CommonAPI.Utils do
require Logger require Logger
require Pleroma.Constants require Pleroma.Constants
def attachments_from_ids(%{media_ids: ids}) do def attachments_from_ids(user, %{media_ids: ids}) do
attachments_from_ids(ids) attachments_from_ids(user, ids, [])
end end
def attachments_from_ids([]), do: [] def attachments_from_ids(_, _), do: []
def attachments_from_ids(ids) when is_list(ids) do defp attachments_from_ids(_user, [], acc), do: Enum.reverse(acc)
Enum.map(ids, fn media_id ->
case get_attachment(media_id) do defp attachments_from_ids(user, [media_id | ids], acc) do
%Object{data: data} -> data with {_, %Object{} = object} <- {:get, get_attachment(media_id)},
_ -> nil :ok <- Object.authorize_access(object, user) do
end attachments_from_ids(user, ids, [object.data | acc])
end) else
|> Enum.reject(&is_nil/1) {:get, _} -> attachments_from_ids(user, ids, acc)
{:error, reason} -> {:error, reason}
end
end end
def attachments_from_ids(_), do: []
defp get_attachment(media_id) do defp get_attachment(media_id) do
Repo.get(Object, media_id) Repo.get(Object, media_id)
end end

View file

@ -592,12 +592,14 @@ defmodule Pleroma.Web.CommonAPI.UtilsTest do
describe "attachments_from_ids/1" do describe "attachments_from_ids/1" do
test "returns attachments without descs" do test "returns attachments without descs" do
object = insert(:note) user = insert(:user)
assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}) == [object.data] object = insert(:note, user: user)
assert Utils.attachments_from_ids(user, %{media_ids: ["#{object.id}"]}) == [object.data]
end end
test "returns [] when not pass media_ids" do test "returns [] when not pass media_ids" do
assert Utils.attachments_from_ids(%{}) == [] user = insert(:user)
assert Utils.attachments_from_ids(user, %{}) == []
end end
end end

View file

@ -220,6 +220,28 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do
assert json_response_and_validate_schema(conn, 200) assert json_response_and_validate_schema(conn, 200)
end end
test "refuses to post non-owned media", %{conn: conn} do
other_user = insert(:user)
file = %Plug.Upload{
content_type: "image/jpeg",
path: Path.absname("test/fixtures/image.jpg"),
filename: "an_image.jpg"
}
{:ok, upload} = ActivityPub.upload(file, actor: other_user.ap_id)
conn =
conn
|> put_req_header("content-type", "application/json")
|> post("/api/v1/statuses", %{
"status" => "mew",
"media_ids" => [to_string(upload.id)]
})
assert json_response(conn, 422) == %{"error" => "forbidden"}
end
test "posting a status with an invalid language", %{conn: conn} do test "posting a status with an invalid language", %{conn: conn} do
conn = conn =
conn conn
@ -569,6 +591,29 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do
assert %{"type" => "image"} = media_attachment assert %{"type" => "image"} = media_attachment
end end
test "refuses to schedule post with non-owned media", %{conn: conn} do
other_user = insert(:user)
file = %Plug.Upload{
content_type: "image/jpeg",
path: Path.absname("test/fixtures/image.jpg"),
filename: "an_image.jpg"
}
{:ok, upload} = ActivityPub.upload(file, actor: other_user.ap_id)
conn =
conn
|> put_req_header("content-type", "application/json")
|> post("/api/v1/statuses", %{
"status" => "mew",
"scheduled_at" => DateTime.add(DateTime.utc_now(), 6, :minute),
"media_ids" => [to_string(upload.id)]
})
assert json_response(conn, 403) == %{"error" => "Access denied"}
end
test "skips the scheduling and creates the activity if scheduled_at is earlier than 5 minutes from now", test "skips the scheduling and creates the activity if scheduled_at is earlier than 5 minutes from now",
%{conn: conn} do %{conn: conn} do
scheduled_at = scheduled_at =
@ -2406,6 +2451,25 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do
assert [%{"id" => ^attachment_id}] = response["media_attachments"] assert [%{"id" => ^attachment_id}] = response["media_attachments"]
end end
test "it does not update to non-owned attachments", %{conn: conn, user: user} do
other_user = insert(:user)
attachment = insert(:attachment, user: other_user)
attachment_id = to_string(attachment.id)
{:ok, activity} = CommonAPI.post(user, %{status: "mew mew #abc", spoiler_text: "#def"})
conn =
conn
|> put_req_header("content-type", "application/json")
|> put("/api/v1/statuses/#{activity.id}", %{
"status" => "mew mew #abc",
"spoiler_text" => "#def",
"media_ids" => [attachment_id]
})
assert json_response(conn, 400) == %{"error" => "internal_server_error"}
end
test "it does not update visibility", %{conn: conn, user: user} do test "it does not update visibility", %{conn: conn, user: user} do
{:ok, activity} = {:ok, activity} =
CommonAPI.post(user, %{ CommonAPI.post(user, %{

View file

@ -47,8 +47,7 @@ defmodule Pleroma.Web.MastodonAPI.ScheduledActivityViewTest do
expected = %{ expected = %{
id: to_string(scheduled_activity.id), id: to_string(scheduled_activity.id),
media_attachments: media_attachments:
%{media_ids: [upload.id]} Utils.attachments_from_ids(user, %{media_ids: [upload.id]})
|> Utils.attachments_from_ids()
|> Enum.map(&StatusView.render("attachment.json", %{attachment: &1})), |> Enum.map(&StatusView.render("attachment.json", %{attachment: &1})),
params: %{ params: %{
in_reply_to_id: to_string(activity.id), in_reply_to_id: to_string(activity.id),