From 05f8179d08197f61b67830ef465e7f6590d6f9cf Mon Sep 17 00:00:00 2001 From: Floatingghost Date: Fri, 12 Apr 2024 05:16:47 +0100 Subject: [PATCH] check if data is visible before embedding it in OG tags previously we would uncritically take data and format it into tags for static-fe and the like - however, instances can be configured to disallow unauthenticated access to these resources. this means that OG tags as a vector for information leakage. _technically_ this should only occur if you have both restrict_unauthenticated *AND* you run static-fe, which makes no sense since static-fe is for unauthenticated people in particular, but hey ho. --- CHANGELOG.md | 1 + .../web/metadata/providers/open_graph.ex | 49 +++++++++++++------ .../web/metadata/providers/twitter_card.ex | 39 +++++++++++---- lib/pleroma/web/metadata/utils.ex | 9 ++++ .../metadata/providers/open_graph_test.exs | 22 +++++++++ .../metadata/providers/twitter_card_test.exs | 20 ++++++++ 6 files changed, 115 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6d04bcb3..699aa30e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Fixed - Issue preventing fetching anything from IPv6-only instances +- Issue allowing post content to leak via opengraph tags despite :estrict\_unauthenticated being set ## 2024.03 diff --git a/lib/pleroma/web/metadata/providers/open_graph.ex b/lib/pleroma/web/metadata/providers/open_graph.ex index df0cca74a..27e761bc2 100644 --- a/lib/pleroma/web/metadata/providers/open_graph.ex +++ b/lib/pleroma/web/metadata/providers/open_graph.ex @@ -12,14 +12,38 @@ defmodule Pleroma.Web.Metadata.Providers.OpenGraph do @behaviour Provider @media_types ["image", "audio", "video"] + defp user_avatar_tags(user) do + if Utils.visible?(user) do + [ + {:meta, [property: "og:image", content: MediaProxy.preview_url(User.avatar_url(user))], + []}, + {:meta, [property: "og:image:width", content: 150], []}, + {:meta, [property: "og:image:height", content: 150], []} + ] + else + [] + end + end + @impl Provider def build_tags(%{ object: object, url: url, user: user }) do - attachments = build_attachments(object) - scrubbed_content = Utils.scrub_html_and_truncate(object) + attachments = + if Utils.visible?(object) do + build_attachments(object) + else + [] + end + + scrubbed_content = + if Utils.visible?(object) do + Utils.scrub_html_and_truncate(object) + else + "Content cannot be displayed." + end [ {:meta, @@ -36,12 +60,7 @@ defmodule Pleroma.Web.Metadata.Providers.OpenGraph do {:meta, [property: "og:type", content: "article"], []} ] ++ if attachments == [] or Metadata.activity_nsfw?(object) do - [ - {:meta, [property: "og:image", content: MediaProxy.preview_url(User.avatar_url(user))], - []}, - {:meta, [property: "og:image:width", content: 150], []}, - {:meta, [property: "og:image:height", content: 150], []} - ] + user_avatar_tags(user) else attachments end @@ -49,7 +68,9 @@ defmodule Pleroma.Web.Metadata.Providers.OpenGraph do @impl Provider def build_tags(%{user: user}) do - with truncated_bio = Utils.scrub_html_and_truncate(user.bio) do + if Utils.visible?(user) do + truncated_bio = Utils.scrub_html_and_truncate(user.bio) + [ {:meta, [ @@ -58,12 +79,10 @@ defmodule Pleroma.Web.Metadata.Providers.OpenGraph do ], []}, {:meta, [property: "og:url", content: user.uri || user.ap_id], []}, {:meta, [property: "og:description", content: truncated_bio], []}, - {:meta, [property: "og:type", content: "article"], []}, - {:meta, [property: "og:image", content: MediaProxy.preview_url(User.avatar_url(user))], - []}, - {:meta, [property: "og:image:width", content: 150], []}, - {:meta, [property: "og:image:height", content: 150], []} - ] + {:meta, [property: "og:type", content: "article"], []} + ] ++ user_avatar_tags(user) + else + [] end end diff --git a/lib/pleroma/web/metadata/providers/twitter_card.ex b/lib/pleroma/web/metadata/providers/twitter_card.ex index ab48ea272..c6d8464e7 100644 --- a/lib/pleroma/web/metadata/providers/twitter_card.ex +++ b/lib/pleroma/web/metadata/providers/twitter_card.ex @@ -17,8 +17,19 @@ defmodule Pleroma.Web.Metadata.Providers.TwitterCard do @impl Provider def build_tags(%{activity_id: id, object: object, user: user}) do - attachments = build_attachments(id, object) - scrubbed_content = Utils.scrub_html_and_truncate(object) + attachments = + if Utils.visible?(object) do + build_attachments(id, object) + else + [] + end + + scrubbed_content = + if Utils.visible?(object) do + Utils.scrub_html_and_truncate(object) + else + "Content cannot be displayed." + end [ title_tag(user), @@ -36,13 +47,17 @@ defmodule Pleroma.Web.Metadata.Providers.TwitterCard do @impl Provider def build_tags(%{user: user}) do - with truncated_bio = Utils.scrub_html_and_truncate(user.bio) do - [ - title_tag(user), - {:meta, [name: "twitter:description", content: truncated_bio], []}, - image_tag(user), - {:meta, [name: "twitter:card", content: "summary"], []} - ] + if Utils.visible?(user) do + with truncated_bio = Utils.scrub_html_and_truncate(user.bio) do + [ + title_tag(user), + {:meta, [name: "twitter:description", content: truncated_bio], []}, + image_tag(user), + {:meta, [name: "twitter:card", content: "summary"], []} + ] + end + else + [] end end @@ -51,7 +66,11 @@ defmodule Pleroma.Web.Metadata.Providers.TwitterCard do end def image_tag(user) do - {:meta, [name: "twitter:image", content: MediaProxy.preview_url(User.avatar_url(user))], []} + if Utils.visible?(user) do + {:meta, [name: "twitter:image", content: MediaProxy.preview_url(User.avatar_url(user))], []} + else + {:meta, [name: "twitter:image", content: ""], []} + end end defp build_attachments(id, %{data: %{"attachment" => attachments}}) do diff --git a/lib/pleroma/web/metadata/utils.ex b/lib/pleroma/web/metadata/utils.ex index 92622ef15..55855139b 100644 --- a/lib/pleroma/web/metadata/utils.ex +++ b/lib/pleroma/web/metadata/utils.ex @@ -7,6 +7,15 @@ defmodule Pleroma.Web.Metadata.Utils do alias Pleroma.Emoji alias Pleroma.Formatter alias Pleroma.HTML + alias Pleroma.Web.ActivityPub.Visibility + + def visible?(%Pleroma.User{} = object) do + Visibility.restrict_unauthenticated_access?(object) == :visible + end + + def visible?(object) do + Visibility.visible_for_user?(object, nil) + end defp scrub_html_and_truncate_object_field(field, object) do field diff --git a/test/pleroma/web/metadata/providers/open_graph_test.exs b/test/pleroma/web/metadata/providers/open_graph_test.exs index 64ff19561..20afb64e5 100644 --- a/test/pleroma/web/metadata/providers/open_graph_test.exs +++ b/test/pleroma/web/metadata/providers/open_graph_test.exs @@ -9,6 +9,8 @@ defmodule Pleroma.Web.Metadata.Providers.OpenGraphTest do setup do: clear_config([Pleroma.Web.Metadata, :unfurl_nsfw]) setup do: clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + setup do: clear_config([:restrict_unauthenticated, :profiles, :local]) + setup do: clear_config([:restrict_unauthenticated, :activities, :local]) test "it renders all supported types of attachments and skips unknown types" do user = insert(:user) @@ -188,4 +190,24 @@ defmodule Pleroma.Web.Metadata.Providers.OpenGraphTest do "http://localhost:4001/proxy/preview/LzAnlke-l5oZbNzWsrHfprX1rGw/aHR0cHM6Ly9wbGVyb21hLmdvdi9hYm91dC9qdWNoZS53ZWJt/juche.webm" ], []} in result end + + test "it does not render users if profiles are marked as restricted" do + clear_config([:restrict_unauthenticated, :profiles, :local], true) + + user = insert(:user) + + result = OpenGraph.build_tags(%{user: user}) + assert Enum.empty?(result) + end + + test "it does not activities users if they are marked as restricted" do + clear_config([:restrict_unauthenticated, :activities, :local], true) + + user = insert(:user) + note = insert(:note, data: %{"actor" => user.ap_id}) + + result = OpenGraph.build_tags(%{object: note, url: note.data["id"], user: user}) + + assert {:meta, [property: "og:description", content: "Content cannot be displayed."], []} in result + end end diff --git a/test/pleroma/web/metadata/providers/twitter_card_test.exs b/test/pleroma/web/metadata/providers/twitter_card_test.exs index cd3f5eced..13fcd56ef 100644 --- a/test/pleroma/web/metadata/providers/twitter_card_test.exs +++ b/test/pleroma/web/metadata/providers/twitter_card_test.exs @@ -14,6 +14,8 @@ defmodule Pleroma.Web.Metadata.Providers.TwitterCardTest do alias Pleroma.Web.Metadata.Utils setup do: clear_config([Pleroma.Web.Metadata, :unfurl_nsfw]) + setup do: clear_config([:restrict_unauthenticated, :profiles, :local]) + setup do: clear_config([:restrict_unauthenticated, :activities, :local]) test "it renders twitter card for user info" do user = insert(:user, name: "Jimmy Hendriks", bio: "born 19 March 1994") @@ -28,6 +30,14 @@ defmodule Pleroma.Web.Metadata.Providers.TwitterCardTest do ] end + test "it does not render twitter card for user info if it is restricted" do + clear_config([:restrict_unauthenticated, :profiles, :local], true) + user = insert(:user, name: "Jimmy Hendriks", bio: "born 19 March 1994") + res = TwitterCard.build_tags(%{user: user}) + + assert Enum.empty?(res) + end + test "it uses summary twittercard if post has no attachment" do user = insert(:user, name: "Jimmy Hendriks", bio: "born 19 March 1994") {:ok, activity} = CommonAPI.post(user, %{status: "HI"}) @@ -54,6 +64,16 @@ defmodule Pleroma.Web.Metadata.Providers.TwitterCardTest do ] == result end + test "it does not summarise activities if they are marked as restricted" do + clear_config([:restrict_unauthenticated, :activities, :local], true) + user = insert(:user) + note = insert(:note, data: %{"actor" => user.ap_id}) + + result = TwitterCard.build_tags(%{object: note, activity_id: note.data["id"], user: user}) + + assert {:meta, [name: "twitter:description", content: "Content cannot be displayed."], []} in result + end + test "it uses summary as description if post has one" do user = insert(:user, name: "Jimmy Hendriks", bio: "born 19 March 1994") {:ok, activity} = CommonAPI.post(user, %{status: "HI"})