hj/pleroma
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

fix(fetcher): require fetched documents to have the fetched id

Browse source 
The fetcher accepted any document whose id was on the same host as the
fetch URL. Anyone who can place a JSON file on a host, for example
through an upload, could pass it off as another object or actor from
that host.

The id of a fetched document must now be the URL it came from (after
redirects), ignoring a trailing slash, the fragment, the host case and
differences that our URL encoding removes.
If it differs, as for Mastodon's /@user/123 display URLs, we refetch
once from that id, which must be on the same host. The refetch must
not redirect and must match exactly.

Test mocks that served a document under a different URL now serve it
from its own id.

Based on Akkoma 8684964c5 and 940792f8b.
This commit is contained in:
Lain Soykaf 2026-10-27 21:45:14 +04:00
commit 20354e0aee
No known key found for this signature in database
8 changed files with 259 additions and 15 deletions
Download patch file Download diff file Expand all files Collapse all files

1
changelog.d/fetch-id-containment.security Normal file
View file

@ -0,0 +1 @@
Only accept fetched objects whose id is the URL they were fetched from; for other ids, refetch once from the canonical id on the same host

28
lib/pleroma/object/containment.ex
View file

@ -108,6 +108,34 @@ defmodule Pleroma.Object.Containment do
def contain_origin_from_id(_id, _data), do: :error
@doc """
Checks that a fetched document's id is the URL it was fetched from
(after redirects).
Fragments, a trailing slash, the case of the host and differences
that our URL encoding removes before a request are ignored.
Documents without an id never match.
Based on Akkoma's `contain_id_to_fetch/2`.
"""
def contain_id_to_fetch(url, %{"id" => id}) when is_binary(url) and is_binary(id) do
if normalize_uri(url) == normalize_uri(id), do: :ok, else: :error
end
def contain_id_to_fetch(_url, _data), do: :error
defp normalize_uri(uri) do
uri = uri |> Pleroma.Utils.URIEncoding.encode_url() |> URI.parse()
%URI{
uri
| fragment: nil,
host: uri.host && String.downcase(uri.host, :ascii),
path: uri.path && String.replace_suffix(uri.path, "/", "")
}
|> URI.to_string()
end
def contain_child(%{"object" => %{"id" => id, "attributedTo" => _} = object}),
do: contain_origin(id, object)

35
lib/pleroma/object/fetcher.ex
View file

@ -140,17 +140,39 @@ defmodule Pleroma.Object.Fetcher do
def fetch_and_contain_remote_object_from_id(%{"id" => id}),
do: fetch_and_contain_remote_object_from_id(id)
def fetch_and_contain_remote_object_from_id(id) when is_binary(id) do
def fetch_and_contain_remote_object_from_id(id) when is_binary(id),
do: fetch_and_contain(id, false)
def fetch_and_contain_remote_object_from_id(_id),
do: {:error, "id must be a string"}
# The fetched document's id must be the URL it came from. Display URLs
# (e.g. Mastodon's /@user/123) serve the object under another id, so
# if the id differs we refetch once from that id, which has to be on the
# same host, must not redirect, and must then match exactly.
# Based on Akkoma 8684964c5 and 940792f8b.
defp fetch_and_contain(id, canonical?) do
Logger.debug("Fetching object #{id} via AP")
with {:scheme, true} <- {:scheme, String.starts_with?(id, "http")},
{_, true} <- {:mrf, MRF.id_filter(id)},
{_, :ok} <- {:local_fetch, Containment.contain_local_fetch(id)},
{:ok, body} <- get_object(id),
{:ok, final_url, body} <- get_object(id),
{_, true} <- {:redirect, not canonical? or same_url?(final_url, id)},
{:ok, data} <- safe_json_decode(body),
:ok <- Containment.contain_origin_from_id(id, data) do
:ok <- Containment.contain_origin_from_id(id, data),
{_, _, :ok} <- {:id, data, Containment.contain_id_to_fetch(final_url, data)} do
{:ok, data}
else
{:id, %{"id" => canonical_id}, :error} when is_binary(canonical_id) and not canonical? ->
fetch_and_contain(canonical_id, true)
{:id, _, :error} ->
{:error, :id_mismatch}
{:redirect, false} ->
{:error, :canonical_id_redirect}
{:scheme, _} ->
{:error, "Unsupported URI scheme"}
@ -168,8 +190,7 @@ defmodule Pleroma.Object.Fetcher do
end
end
def fetch_and_contain_remote_object_from_id(_id),
do: {:error, "id must be a string"}
defp same_url?(url, other), do: Containment.contain_id_to_fetch(url, %{"id" => other}) == :ok
defp check_crossdomain_redirect(final_host, _original_url) when is_nil(final_host) do
{:cross_domain_redirect, false}
@ -195,7 +216,7 @@ defmodule Pleroma.Object.Fetcher do
with {:cross_domain_redirect, false} <- check_crossdomain_redirect(remote_host, id),
{_, content_type} <- List.keyfind(headers, "content-type", 0),
{:ok, _media_type} <- verify_content_type(content_type) do
{:ok, body}
{:ok, if(final_url in [nil, ""], do: id, else: final_url), body}
else
{:cross_domain_redirect, true} ->
{:error, {:cross_domain_redirect, true}}
@ -209,7 +230,7 @@ defmodule Pleroma.Object.Fetcher do
case List.keyfind(headers, "content-type", 0) do
{_, content_type} ->
case verify_content_type(content_type) do
{:ok, _} -> {:ok, body}
{:ok, _} -> {:ok, id, body}
error -> error
end

6
test/fixtures/tesla_mock/relay@mastdon.example.org.json vendored
View file

@ -11,7 +11,7 @@
"toot": "http://joinmastodon.org/ns#",
"Emoji": "toot:Emoji"
}],
"id": "http://mastodon.example.org/users/admin",
"id": "http://mastodon.example.org/users/relay",
"type": "Application",
"invisible": true,
"following": "http://mastodon.example.org/users/admin/following",
@ -24,8 +24,8 @@
"url": "http://mastodon.example.org/@admin",
"manuallyApprovesFollowers": false,
"publicKey": {
"id": "http://mastodon.example.org/users/admin#main-key",
"owner": "http://mastodon.example.org/users/admin",
"id": "http://mastodon.example.org/users/relay#main-key",
"owner": "http://mastodon.example.org/users/relay",
"publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAtc4Tir+3ADhSNF6VKrtW\nOU32T01w7V0yshmQei38YyiVwVvFu8XOP6ACchkdxbJ+C9mZud8qWaRJKVbFTMUG\nNX4+6Q+FobyuKrwN7CEwhDALZtaN2IPbaPd6uG1B7QhWorrY+yFa8f2TBM3BxnUy\nI4T+bMIZIEYG7KtljCBoQXuTQmGtuffO0UwJksidg2ffCF5Q+K//JfQagJ3UzrR+\nZXbKMJdAw4bCVJYs4Z5EhHYBwQWiXCyMGTd7BGlmMkY6Av7ZqHKC/owp3/0EWDNz\nNqF09Wcpr3y3e8nA10X40MJqp/wR+1xtxp+YGbq/Cj5hZGBG7etFOmIpVBrDOhry\nBwIDAQAB\n-----END PUBLIC KEY-----\n"
},
"attachment": [{

34
test/pleroma/object/containment_test.exs
View file

@ -127,4 +127,38 @@ defmodule Pleroma.Object.ContainmentTest do
:ok = Containment.contain_child(data)
end
end
describe "contain_id_to_fetch/2" do
test "matches the exact id" do
assert :ok =
Containment.contain_id_to_fetch("https://a.test/o/1", %{
"id" => "https://a.test/o/1"
})
end
test "ignores fragments, a trailing slash and the host case" do
assert :ok =
Containment.contain_id_to_fetch("https://A.test/o/1/", %{
"id" => "https://a.test/o/1#x"
})
end
test "rejects another id on the same host" do
assert :error =
Containment.contain_id_to_fetch("https://a.test/o/1", %{
"id" => "https://a.test/o/2"
})
end
test "rejects a different query" do
assert :error =
Containment.contain_id_to_fetch("https://a.test/o?page=1", %{
"id" => "https://a.test/o?page=2"
})
end
test "rejects documents without an id" do
assert :error = Containment.contain_id_to_fetch("https://a.test/o/1", %{})
end
end
end

154
test/pleroma/object/fetcher_test.exs
View file

@ -48,6 +48,8 @@ defmodule Pleroma.Object.FetcherTest do
%{method: :get, url: "https://social.sakamoto.gq/notice/9wTkLEnuq47B25EehM"} ->
%Tesla.Env{
status: 200,
# Pleroma redirects /notice/:id to the object id
url: "https://social.sakamoto.gq/objects/f20f2497-66d9-4a52-a2e1-1be2a39c32c1",
body: File.read!("test/fixtures/fetch_mocks/9wTkLEnuq47B25EehM.json"),
headers: HttpRequestMock.activitypub_object_headers()
}
@ -642,7 +644,23 @@ defmodule Pleroma.Object.FetcherTest do
})
}
# No redirect - same domain
# Same-domain redirect to the object id
%{method: :get, url: "https://original.test/@actor/redirect"} ->
%Tesla.Env{
status: 200,
url: "https://original.test/objects/redirect-target",
headers: [{"content-type", "application/activity+json"}],
body:
Jason.encode!(%{
"id" => "https://original.test/objects/redirect-target",
"type" => "Note",
"content" => "This has a same-domain redirect",
"actor" => "https://original.test/users/actor",
"attributedTo" => "https://original.test/users/actor"
})
}
# Same-domain redirect to a document claiming the original URL as id
%{method: :get, url: "https://original.test/objects/same-domain-redirect"} ->
%Tesla.Env{
status: 200,
@ -695,12 +713,19 @@ defmodule Pleroma.Object.FetcherTest do
test "it accepts objects with same-domain redirects" do
assert {:ok, data} =
Fetcher.fetch_and_contain_remote_object_from_id(
"https://original.test/objects/same-domain-redirect"
"https://original.test/@actor/redirect"
)
assert data["content"] == "This has a same-domain redirect"
end
test "it rejects redirect targets that claim the original URL as id" do
assert {:error, _} =
Fetcher.fetch_and_contain_remote_object_from_id(
"https://original.test/objects/same-domain-redirect"
)
end
test "it handles responses without URL field (common in tests)" do
assert {:ok, data} =
Fetcher.fetch_and_contain_remote_object_from_id(
@ -711,6 +736,131 @@ defmodule Pleroma.Object.FetcherTest do
end
end
describe "id containment" do
setup do
note = fn id, content ->
%Tesla.Env{
status: 200,
headers: [{"content-type", "application/activity+json"}],
body:
Jason.encode!(%{
"id" => id,
"type" => "Note",
"content" => content,
"actor" => "https://host.test/users/actor",
"attributedTo" => "https://host.test/users/actor"
})
}
end
mock(fn
# display URL serving the canonical object
%{method: :get, url: "https://host.test/@actor/1"} ->
note.("https://host.test/objects/1", "display")
%{method: :get, url: "https://host.test/objects/1"} ->
note.("https://host.test/objects/1", "canonical")
# uploaded file claiming to be another object on the same host
%{method: :get, url: "https://host.test/media/evil.json"} ->
note.("https://host.test/objects/2", "spoofed")
%{method: :get, url: "https://host.test/objects/2"} ->
note.("https://host.test/objects/2", "real")
# canonical id does not serve itself
%{method: :get, url: "https://host.test/media/loop.json"} ->
note.("https://host.test/objects/3", "loop")
%{method: :get, url: "https://host.test/objects/3"} ->
note.("https://host.test/objects/4", "other")
# canonical id redirects
%{method: :get, url: "https://host.test/media/redirect.json"} ->
note.("https://host.test/objects/5", "redirect")
%{method: :get, url: "https://host.test/objects/5"} ->
%{note.("https://host.test/objects/5", "moved") | url: "https://host.test/media/x"}
%{method: :get, url: "https://host.test/objects/slash"} ->
note.("https://host.test/objects/slash/", "slash")
%{method: :get, url: "https://host.test/objects/no-id"} ->
note.(nil, "no id")
end)
:ok
end
test "it refetches display URLs from the canonical id" do
assert {:ok, %{"id" => "https://host.test/objects/1", "content" => "canonical"}} =
Fetcher.fetch_and_contain_remote_object_from_id("https://host.test/@actor/1")
end
test "it does not trust a document claiming another id on the same host" do
assert {:ok, %{"id" => "https://host.test/objects/2", "content" => "real"}} =
Fetcher.fetch_and_contain_remote_object_from_id(
"https://host.test/media/evil.json"
)
end
test "it refetches at most once" do
assert {:error, :id_mismatch} =
Fetcher.fetch_and_contain_remote_object_from_id(
"https://host.test/media/loop.json"
)
end
test "it does not follow redirects when refetching from the canonical id" do
assert {:error, _} =
Fetcher.fetch_and_contain_remote_object_from_id(
"https://host.test/media/redirect.json"
)
end
test "it ignores a trailing slash" do
assert {:ok, %{"content" => "slash"}} =
Fetcher.fetch_and_contain_remote_object_from_id("https://host.test/objects/slash")
end
test "it accepts ids that only differ by URL encoding" do
clear_config(:test_url_encoding, true)
mock(fn %{method: :get, url: url} ->
# the encoder sends uppercase escapes
assert url in [
"https://host.test/users/%E3%81%82",
"https://host.test/notes/%E3%81%82"
]
id =
if url =~ "users",
do: "https://host.test/users/あ",
else: "https://host.test/notes/%e3%81%82"
%Tesla.Env{
status: 200,
url: url,
headers: [{"content-type", "application/activity+json"}],
body: Jason.encode!(%{"id" => id, "type" => "Note"})
}
end)
assert {:ok, _} =
Fetcher.fetch_and_contain_remote_object_from_id("https://host.test/users/あ")
assert {:ok, _} =
Fetcher.fetch_and_contain_remote_object_from_id(
"https://host.test/notes/%e3%81%82"
)
end
test "it rejects documents without an id" do
assert {:error, _} =
Fetcher.fetch_and_contain_remote_object_from_id("https://host.test/objects/no-id")
end
end
describe "fetch with history" do
setup do
object2 = %{

6
test/pleroma/web/activity_pub/activity_pub_test.exs
View file

@ -541,7 +541,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
end
test "fetches user birthday information from misskey" do
user_id = "https://misskey.io/@mkljczk"
user_id = "https://misskey.io/users/8dhi2ne167"
Tesla.Mock.mock(fn
%{
@ -2058,7 +2058,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
json(
%{
"@context" => "https://www.w3.org/ns/activitystreams",
"id" => "https://remote.org/users/masto_hidden_counters/followers"
"id" => "https://remote.org/users/masto_hidden_counters/following"
},
headers: HttpRequestMock.activitypub_object_headers()
)
@ -2070,7 +2070,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
json(
%{
"@context" => "https://www.w3.org/ns/activitystreams",
"id" => "https://remote.org/users/masto_hidden_counters/following"
"id" => "https://remote.org/users/masto_hidden_counters/followers"
},
headers: HttpRequestMock.activitypub_object_headers()
)

10
test/support/http_request_mock.ex
View file

@ -577,6 +577,16 @@ defmodule HttpRequestMock do
}}
end
# the same status via its canonical id
def get("http://mastodon.example.org/users/admin/statuses/99541947525187367", _, _, _) do
{:ok,
%Tesla.Env{
status: 200,
body: File.read!("test/fixtures/mastodon-note-object.json"),
headers: activitypub_object_headers()
}}
end
def get("http://mastodon.example.org/@admin/99541947525187368", _, _, _) do
{:ok,
%Tesla.Env{