From 59a142e0b0e6760e116443567802ef2ab7483178 Mon Sep 17 00:00:00 2001 From: Oneric Date: Wed, 13 Mar 2024 21:00:23 -0100 Subject: [PATCH] Never fetch resource from ourselves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If it’s not already in the database, it must be counterfeit (or just not exists at all) Changed test URLs were only ever used from "local: false" users anyway. --- lib/pleroma/object/containment.ex | 13 +++++++ lib/pleroma/object/fetcher.ex | 4 +++ .../users_mock/friendica_followers.json | 2 +- .../users_mock/friendica_following.json | 2 +- .../users_mock/masto_closed_followers.json | 4 +-- .../masto_closed_followers_page.json | 2 +- .../users_mock/masto_closed_following.json | 4 +-- .../masto_closed_following_page.json | 2 +- .../users_mock/pleroma_followers.json | 8 ++--- .../users_mock/pleroma_following.json | 8 ++--- test/pleroma/object/fetcher_test.exs | 7 ++++ test/pleroma/user_test.exs | 22 ++++++------ .../web/activity_pub/activity_pub_test.exs | 36 +++++++++---------- test/support/http_request_mock.ex | 16 ++++----- 14 files changed, 77 insertions(+), 53 deletions(-) diff --git a/lib/pleroma/object/containment.ex b/lib/pleroma/object/containment.ex index 85b777179..48e62a917 100644 --- a/lib/pleroma/object/containment.ex +++ b/lib/pleroma/object/containment.ex @@ -47,6 +47,19 @@ def get_object(_) do defp compare_uris(%URI{host: host} = _id_uri, %URI{host: host} = _other_uri), do: :ok defp compare_uris(_id_uri, _other_uri), do: :error + @doc """ + Checks whether an URL to fetch from is from the local server. + + We never want to fetch from ourselves; if it’s not in the database + it can’t be authentic and must be a counterfeit. + """ + def contain_local_fetch(id) do + case compare_uris(URI.parse(id), Pleroma.Web.Endpoint.struct_url()) do + :ok -> :error + _ -> :ok + end + end + @doc """ Checks that an imported AP object's actor matches the host it came from. """ diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 16d8194e7..f256bbf77 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -258,6 +258,7 @@ def fetch_and_contain_remote_object_from_id(id) when is_binary(id) do Logger.debug("Fetching object #{id} via AP") with {:scheme, true} <- {:scheme, String.starts_with?(id, "http")}, + {_, :ok} <- {:local_fetch, Containment.contain_local_fetch(id)}, {:ok, body} <- get_object(id), {:ok, data} <- safe_json_decode(body), {_, :ok} <- {:containment, Containment.contain_origin_from_id(id, data)}, @@ -271,6 +272,9 @@ def fetch_and_contain_remote_object_from_id(id) when is_binary(id) do {:scheme, _} -> {:error, "Unsupported URI scheme"} + {:local_fetch, _} -> + {:error, "Trying to fetch local resource"} + {:containment, _} -> {:error, "Object containment failed."} diff --git a/test/fixtures/users_mock/friendica_followers.json b/test/fixtures/users_mock/friendica_followers.json index 7b86b5fe2..02b287e23 100644 --- a/test/fixtures/users_mock/friendica_followers.json +++ b/test/fixtures/users_mock/friendica_followers.json @@ -13,7 +13,7 @@ "directMessage": "litepub:directMessage" } ], - "id": "http://localhost:8080/followers/fuser3", + "id": "http://remote.org/followers/fuser3", "type": "OrderedCollection", "totalItems": 296 } diff --git a/test/fixtures/users_mock/friendica_following.json b/test/fixtures/users_mock/friendica_following.json index 7c526befc..0908e78f0 100644 --- a/test/fixtures/users_mock/friendica_following.json +++ b/test/fixtures/users_mock/friendica_following.json @@ -13,7 +13,7 @@ "directMessage": "litepub:directMessage" } ], - "id": "http://localhost:8080/following/fuser3", + "id": "http://remote.org/following/fuser3", "type": "OrderedCollection", "totalItems": 32 } diff --git a/test/fixtures/users_mock/masto_closed_followers.json b/test/fixtures/users_mock/masto_closed_followers.json index da296892d..ccc32d15e 100644 --- a/test/fixtures/users_mock/masto_closed_followers.json +++ b/test/fixtures/users_mock/masto_closed_followers.json @@ -1,7 +1,7 @@ { "@context": "https://www.w3.org/ns/activitystreams", - "id": "http://localhost:4001/users/masto_closed/followers", + "id": "http://remote.org/users/masto_closed/followers", "type": "OrderedCollection", "totalItems": 437, - "first": "http://localhost:4001/users/masto_closed/followers?page=1" + "first": "http://remote.org/users/masto_closed/followers?page=1" } diff --git a/test/fixtures/users_mock/masto_closed_followers_page.json b/test/fixtures/users_mock/masto_closed_followers_page.json index 04ab0c4d3..e4f1b3ac0 100644 --- a/test/fixtures/users_mock/masto_closed_followers_page.json +++ b/test/fixtures/users_mock/masto_closed_followers_page.json @@ -1 +1 @@ -{"@context":"https://www.w3.org/ns/activitystreams","id":"http://localhost:4001/users/masto_closed/followers?page=1","type":"OrderedCollectionPage","totalItems":437,"next":"http://localhost:4001/users/masto_closed/followers?page=2","partOf":"http://localhost:4001/users/masto_closed/followers","orderedItems":["https://testing.uguu.ltd/users/rin","https://patch.cx/users/rin","https://letsalllovela.in/users/xoxo","https://pleroma.site/users/crushv","https://aria.company/users/boris","https://kawen.space/users/crushv","https://freespeech.host/users/cvcvcv","https://pleroma.site/users/picpub","https://pixelfed.social/users/nosleep","https://boopsnoot.gq/users/5c1896d162f7d337f90492a3","https://pikachu.rocks/users/waifu","https://royal.crablettesare.life/users/crablettes"]} +{"@context":"https://www.w3.org/ns/activitystreams","id":"http://remote.org/users/masto_closed/followers?page=1","type":"OrderedCollectionPage","totalItems":437,"next":"http://remote.org/users/masto_closed/followers?page=2","partOf":"http://remote.org/users/masto_closed/followers","orderedItems":["https://testing.uguu.ltd/users/rin","https://patch.cx/users/rin","https://letsalllovela.in/users/xoxo","https://pleroma.site/users/crushv","https://aria.company/users/boris","https://kawen.space/users/crushv","https://freespeech.host/users/cvcvcv","https://pleroma.site/users/picpub","https://pixelfed.social/users/nosleep","https://boopsnoot.gq/users/5c1896d162f7d337f90492a3","https://pikachu.rocks/users/waifu","https://royal.crablettesare.life/users/crablettes"]} diff --git a/test/fixtures/users_mock/masto_closed_following.json b/test/fixtures/users_mock/masto_closed_following.json index 146d49f9c..34e9e9fe8 100644 --- a/test/fixtures/users_mock/masto_closed_following.json +++ b/test/fixtures/users_mock/masto_closed_following.json @@ -1,7 +1,7 @@ { "@context": "https://www.w3.org/ns/activitystreams", - "id": "http://localhost:4001/users/masto_closed/following", + "id": "http://remote.org/users/masto_closed/following", "type": "OrderedCollection", "totalItems": 152, - "first": "http://localhost:4001/users/masto_closed/following?page=1" + "first": "http://remote.org/users/masto_closed/following?page=1" } diff --git a/test/fixtures/users_mock/masto_closed_following_page.json b/test/fixtures/users_mock/masto_closed_following_page.json index 8d8324699..d398ae3cf 100644 --- a/test/fixtures/users_mock/masto_closed_following_page.json +++ b/test/fixtures/users_mock/masto_closed_following_page.json @@ -1 +1 @@ -{"@context":"https://www.w3.org/ns/activitystreams","id":"http://localhost:4001/users/masto_closed/following?page=1","type":"OrderedCollectionPage","totalItems":152,"next":"http://localhost:4001/users/masto_closed/following?page=2","partOf":"http://localhost:4001/users/masto_closed/following","orderedItems":["https://testing.uguu.ltd/users/rin","https://patch.cx/users/rin","https://letsalllovela.in/users/xoxo","https://pleroma.site/users/crushv","https://aria.company/users/boris","https://kawen.space/users/crushv","https://freespeech.host/users/cvcvcv","https://pleroma.site/users/picpub","https://pixelfed.social/users/nosleep","https://boopsnoot.gq/users/5c1896d162f7d337f90492a3","https://pikachu.rocks/users/waifu","https://royal.crablettesare.life/users/crablettes"]} +{"@context":"https://www.w3.org/ns/activitystreams","id":"http://remote.org/users/masto_closed/following?page=1","type":"OrderedCollectionPage","totalItems":152,"next":"http://remote.org/users/masto_closed/following?page=2","partOf":"http://remote.org/users/masto_closed/following","orderedItems":["https://testing.uguu.ltd/users/rin","https://patch.cx/users/rin","https://letsalllovela.in/users/xoxo","https://pleroma.site/users/crushv","https://aria.company/users/boris","https://kawen.space/users/crushv","https://freespeech.host/users/cvcvcv","https://pleroma.site/users/picpub","https://pixelfed.social/users/nosleep","https://boopsnoot.gq/users/5c1896d162f7d337f90492a3","https://pikachu.rocks/users/waifu","https://royal.crablettesare.life/users/crablettes"]} diff --git a/test/fixtures/users_mock/pleroma_followers.json b/test/fixtures/users_mock/pleroma_followers.json index db71d084b..9611990ee 100644 --- a/test/fixtures/users_mock/pleroma_followers.json +++ b/test/fixtures/users_mock/pleroma_followers.json @@ -1,14 +1,14 @@ { "type": "OrderedCollection", "totalItems": 527, - "id": "http://localhost:4001/users/fuser2/followers", + "id": "http://remote.org/users/fuser2/followers", "first": { "type": "OrderedCollectionPage", "totalItems": 527, - "partOf": "http://localhost:4001/users/fuser2/followers", + "partOf": "http://remote.org/users/fuser2/followers", "orderedItems": [], - "next": "http://localhost:4001/users/fuser2/followers?page=2", - "id": "http://localhost:4001/users/fuser2/followers?page=1" + "next": "http://remote.org/users/fuser2/followers?page=2", + "id": "http://remote.org/users/fuser2/followers?page=1" }, "@context": [ "https://www.w3.org/ns/activitystreams", diff --git a/test/fixtures/users_mock/pleroma_following.json b/test/fixtures/users_mock/pleroma_following.json index 33d087703..27fadbc94 100644 --- a/test/fixtures/users_mock/pleroma_following.json +++ b/test/fixtures/users_mock/pleroma_following.json @@ -1,14 +1,14 @@ { "type": "OrderedCollection", "totalItems": 267, - "id": "http://localhost:4001/users/fuser2/following", + "id": "http://remote.org/users/fuser2/following", "first": { "type": "OrderedCollectionPage", "totalItems": 267, - "partOf": "http://localhost:4001/users/fuser2/following", + "partOf": "http://remote.org/users/fuser2/following", "orderedItems": [], - "next": "http://localhost:4001/users/fuser2/following?page=2", - "id": "http://localhost:4001/users/fuser2/following?page=1" + "next": "http://remote.org/users/fuser2/following?page=2", + "id": "http://remote.org/users/fuser2/following?page=1" }, "@context": [ "https://www.w3.org/ns/activitystreams", diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index b289e869d..d1f3c070e 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -237,6 +237,13 @@ test "it does not fetch a spoofed object with a foreign actor" do "https://patch.cx/objects/spoof_foreign_actor" ) end + + test "it does not fetch from localhost" do + assert {:error, "Trying to fetch local resource"} = + Fetcher.fetch_and_contain_remote_object_from_id( + Pleroma.Web.Endpoint.url() <> "/spoof_local" + ) + end end describe "fetching an object" do diff --git a/test/pleroma/user_test.exs b/test/pleroma/user_test.exs index 5a0d77cab..96ca8d0fd 100644 --- a/test/pleroma/user_test.exs +++ b/test/pleroma/user_test.exs @@ -326,9 +326,9 @@ test "unfollow with synchronizes external user" do insert(:user, %{ local: false, nickname: "fuser2", - ap_id: "http://localhost:4001/users/fuser2", - follower_address: "http://localhost:4001/users/fuser2/followers", - following_address: "http://localhost:4001/users/fuser2/following" + ap_id: "http://remote.org/users/fuser2", + follower_address: "http://remote.org/users/fuser2/followers", + following_address: "http://remote.org/users/fuser2/following" }) {:ok, user, followed} = User.follow(user, followed, :follow_accept) @@ -2177,8 +2177,8 @@ test "it returns a list of AP ids for a given set of nicknames" do describe "sync followers count" do setup do - user1 = insert(:user, local: false, ap_id: "http://localhost:4001/users/masto_closed") - user2 = insert(:user, local: false, ap_id: "http://localhost:4001/users/fuser2") + user1 = insert(:user, local: false, ap_id: "http://remote.org/users/masto_closed") + user2 = insert(:user, local: false, ap_id: "http://remote.org/users/fuser2") insert(:user, local: true) insert(:user, local: false, is_active: false) {:ok, user1: user1, user2: user2} @@ -2272,8 +2272,8 @@ test "updates the counters normally on following/getting a follow when disabled" other_user = insert(:user, local: false, - follower_address: "http://localhost:4001/users/masto_closed/followers", - following_address: "http://localhost:4001/users/masto_closed/following", + follower_address: "http://remote.org/users/masto_closed/followers", + following_address: "http://remote.org/users/masto_closed/following", ap_enabled: true ) @@ -2294,8 +2294,8 @@ test "synchronizes the counters with the remote instance for the followed when e other_user = insert(:user, local: false, - follower_address: "http://localhost:4001/users/masto_closed/followers", - following_address: "http://localhost:4001/users/masto_closed/following", + follower_address: "http://remote.org/users/masto_closed/followers", + following_address: "http://remote.org/users/masto_closed/following", ap_enabled: true ) @@ -2316,8 +2316,8 @@ test "synchronizes the counters with the remote instance for the follower when e other_user = insert(:user, local: false, - follower_address: "http://localhost:4001/users/masto_closed/followers", - following_address: "http://localhost:4001/users/masto_closed/following", + follower_address: "http://remote.org/users/masto_closed/followers", + following_address: "http://remote.org/users/masto_closed/following", ap_enabled: true ) diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs index 69b4ac257..c6543ec83 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_test.exs @@ -1643,8 +1643,8 @@ test "synchronizes following/followers counters" do user = insert(:user, local: false, - follower_address: "http://localhost:4001/users/fuser2/followers", - following_address: "http://localhost:4001/users/fuser2/following" + follower_address: "http://remote.org/users/fuser2/followers", + following_address: "http://remote.org/users/fuser2/following" ) {:ok, info} = ActivityPub.fetch_follow_information_for_user(user) @@ -1655,7 +1655,7 @@ test "synchronizes following/followers counters" do test "detects hidden followers" do mock(fn env -> case env.url do - "http://localhost:4001/users/masto_closed/followers?page=1" -> + "http://remote.org/users/masto_closed/followers?page=1" -> %Tesla.Env{status: 403, body: ""} _ -> @@ -1666,8 +1666,8 @@ test "detects hidden followers" do user = insert(:user, local: false, - follower_address: "http://localhost:4001/users/masto_closed/followers", - following_address: "http://localhost:4001/users/masto_closed/following" + follower_address: "http://remote.org/users/masto_closed/followers", + following_address: "http://remote.org/users/masto_closed/following" ) {:ok, follow_info} = ActivityPub.fetch_follow_information_for_user(user) @@ -1678,7 +1678,7 @@ test "detects hidden followers" do test "detects hidden follows" do mock(fn env -> case env.url do - "http://localhost:4001/users/masto_closed/following?page=1" -> + "http://remote.org/users/masto_closed/following?page=1" -> %Tesla.Env{status: 403, body: ""} _ -> @@ -1689,8 +1689,8 @@ test "detects hidden follows" do user = insert(:user, local: false, - follower_address: "http://localhost:4001/users/masto_closed/followers", - following_address: "http://localhost:4001/users/masto_closed/following" + follower_address: "http://remote.org/users/masto_closed/followers", + following_address: "http://remote.org/users/masto_closed/following" ) {:ok, follow_info} = ActivityPub.fetch_follow_information_for_user(user) @@ -1702,8 +1702,8 @@ test "detects hidden follows/followers for friendica" do user = insert(:user, local: false, - follower_address: "http://localhost:8080/followers/fuser3", - following_address: "http://localhost:8080/following/fuser3" + follower_address: "http://remote.org/followers/fuser3", + following_address: "http://remote.org/following/fuser3" ) {:ok, follow_info} = ActivityPub.fetch_follow_information_for_user(user) @@ -1716,28 +1716,28 @@ test "detects hidden follows/followers for friendica" do test "doesn't crash when follower and following counters are hidden" do mock(fn env -> case env.url do - "http://localhost:4001/users/masto_hidden_counters/following" -> + "http://remote.org/users/masto_hidden_counters/following" -> json( %{ "@context" => "https://www.w3.org/ns/activitystreams", - "id" => "http://localhost:4001/users/masto_hidden_counters/followers" + "id" => "http://remote.org/users/masto_hidden_counters/followers" }, headers: HttpRequestMock.activitypub_object_headers() ) - "http://localhost:4001/users/masto_hidden_counters/following?page=1" -> + "http://remote.org/users/masto_hidden_counters/following?page=1" -> %Tesla.Env{status: 403, body: ""} - "http://localhost:4001/users/masto_hidden_counters/followers" -> + "http://remote.org/users/masto_hidden_counters/followers" -> json( %{ "@context" => "https://www.w3.org/ns/activitystreams", - "id" => "http://localhost:4001/users/masto_hidden_counters/following" + "id" => "http://remote.org/users/masto_hidden_counters/following" }, headers: HttpRequestMock.activitypub_object_headers() ) - "http://localhost:4001/users/masto_hidden_counters/followers?page=1" -> + "http://remote.org/users/masto_hidden_counters/followers?page=1" -> %Tesla.Env{status: 403, body: ""} end end) @@ -1745,8 +1745,8 @@ test "doesn't crash when follower and following counters are hidden" do user = insert(:user, local: false, - follower_address: "http://localhost:4001/users/masto_hidden_counters/followers", - following_address: "http://localhost:4001/users/masto_hidden_counters/following" + follower_address: "http://remote.org/users/masto_hidden_counters/followers", + following_address: "http://remote.org/users/masto_hidden_counters/following" ) {:ok, follow_info} = ActivityPub.fetch_follow_information_for_user(user) diff --git a/test/support/http_request_mock.ex b/test/support/http_request_mock.ex index 6772a7421..e831f43f7 100644 --- a/test/support/http_request_mock.ex +++ b/test/support/http_request_mock.ex @@ -964,7 +964,7 @@ def get("https://pleroma.local/notice/9kCP7V", _, _, _) do {:ok, %Tesla.Env{status: 200, body: File.read!("test/fixtures/rich_media/ogp.html")}} end - def get("http://localhost:4001/users/masto_closed/followers", _, _, _) do + def get("http://remote.org/users/masto_closed/followers", _, _, _) do {:ok, %Tesla.Env{ status: 200, @@ -973,7 +973,7 @@ def get("http://localhost:4001/users/masto_closed/followers", _, _, _) do }} end - def get("http://localhost:4001/users/masto_closed/followers?page=1", _, _, _) do + def get("http://remote.org/users/masto_closed/followers?page=1", _, _, _) do {:ok, %Tesla.Env{ status: 200, @@ -982,7 +982,7 @@ def get("http://localhost:4001/users/masto_closed/followers?page=1", _, _, _) do }} end - def get("http://localhost:4001/users/masto_closed/following", _, _, _) do + def get("http://remote.org/users/masto_closed/following", _, _, _) do {:ok, %Tesla.Env{ status: 200, @@ -991,7 +991,7 @@ def get("http://localhost:4001/users/masto_closed/following", _, _, _) do }} end - def get("http://localhost:4001/users/masto_closed/following?page=1", _, _, _) do + def get("http://remote.org/users/masto_closed/following?page=1", _, _, _) do {:ok, %Tesla.Env{ status: 200, @@ -1000,7 +1000,7 @@ def get("http://localhost:4001/users/masto_closed/following?page=1", _, _, _) do }} end - def get("http://localhost:8080/followers/fuser3", _, _, _) do + def get("http://remote.org/followers/fuser3", _, _, _) do {:ok, %Tesla.Env{ status: 200, @@ -1009,7 +1009,7 @@ def get("http://localhost:8080/followers/fuser3", _, _, _) do }} end - def get("http://localhost:8080/following/fuser3", _, _, _) do + def get("http://remote.org/following/fuser3", _, _, _) do {:ok, %Tesla.Env{ status: 200, @@ -1018,7 +1018,7 @@ def get("http://localhost:8080/following/fuser3", _, _, _) do }} end - def get("http://localhost:4001/users/fuser2/followers", _, _, _) do + def get("http://remote.org/users/fuser2/followers", _, _, _) do {:ok, %Tesla.Env{ status: 200, @@ -1027,7 +1027,7 @@ def get("http://localhost:4001/users/fuser2/followers", _, _, _) do }} end - def get("http://localhost:4001/users/fuser2/following", _, _, _) do + def get("http://remote.org/users/fuser2/following", _, _, _) do {:ok, %Tesla.Env{ status: 200,