From 502382da45e88a2b932878cdb0e134a21189ed01 Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Wed, 22 Jun 2022 16:25:05 +0100 Subject: [PATCH] cherry-pick security from upstream --- CHANGELOG.md | 14 ++++++++++- .../activity_pub/activity_pub_controller.ex | 11 +++++++++ lib/pleroma/web/plugs/cache.ex | 12 ++++++++-- mix.exs | 2 +- mix.lock | 2 +- .../activity_pub_controller_test.exs | 24 +++++++++++++++++++ test/pleroma/web/plugs/cache_test.exs | 18 ++++++++++++++ 7 files changed, 78 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9ee14f0e..707d11d71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,7 +64,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Removed -## 2.4.1 - 2021-08-29 +### Security +- Private `/objects/` and `/activities/` leaking if cached by authenticated user +- SweetXML library DTD bomb + +## 2.4.2 - 2022-01-10 + +### Fixed +- Federation issues caused by HTTP pool checkout timeouts +- Compatibility with Elixir 1.13 + +### Upgrade notes + +1. Restart Pleroma ### Changed - Make `mix pleroma.database set_text_search_config` run concurrently and indefinitely diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 4a19938f6..1eb0a3620 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -84,6 +84,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do user <- Map.get(assigns, :user, nil), {_, true} <- {:visible?, Visibility.visible_for_user?(object, user)} do conn + |> maybe_skip_cache(user) |> assign(:tracking_fun_data, object.id) |> set_cache_ttl_for(object) |> put_resp_content_type("application/activity+json") @@ -112,6 +113,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do user <- Map.get(assigns, :user, nil), {_, true} <- {:visible?, Visibility.visible_for_user?(activity, user)} do conn + |> maybe_skip_cache(user) |> maybe_set_tracking_data(activity) |> set_cache_ttl_for(activity) |> put_resp_content_type("application/activity+json") @@ -151,6 +153,15 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do assign(conn, :cache_ttl, ttl) end + def maybe_skip_cache(conn, user) do + if user do + conn + |> assign(:skip_cache, true) + else + conn + end + end + # GET /relay/following def relay_following(conn, _params) do with %{halted: false} = conn <- FederatingPlug.call(conn, []) do diff --git a/lib/pleroma/web/plugs/cache.ex b/lib/pleroma/web/plugs/cache.ex index 111854859..935b2d834 100644 --- a/lib/pleroma/web/plugs/cache.ex +++ b/lib/pleroma/web/plugs/cache.ex @@ -97,13 +97,21 @@ defmodule Pleroma.Web.Plugs.Cache do key = cache_key(conn, opts) content_type = content_type(conn) + should_cache = not Map.get(conn.assigns, :skip_cache, false) + conn = unless opts[:tracking_fun] do - @cachex.put(:web_resp_cache, key, {content_type, body}, ttl: ttl) + if should_cache do + @cachex.put(:web_resp_cache, key, {content_type, body}, ttl: ttl) + end + conn else tracking_fun_data = Map.get(conn.assigns, :tracking_fun_data, nil) - @cachex.put(:web_resp_cache, key, {content_type, body, tracking_fun_data}, ttl: ttl) + + if should_cache do + @cachex.put(:web_resp_cache, key, {content_type, body, tracking_fun_data}, ttl: ttl) + end opts.tracking_fun.(conn, tracking_fun_data) end diff --git a/mix.exs b/mix.exs index 9966c1819..163d498b7 100644 --- a/mix.exs +++ b/mix.exs @@ -141,7 +141,7 @@ defmodule Pleroma.Mixfile do {:mogrify, "~> 0.9.1"}, {:ex_aws, "~> 2.1.6"}, {:ex_aws_s3, "~> 2.0"}, - {:sweet_xml, "~> 0.6.6"}, + {:sweet_xml, "~> 0.7.2"}, {:earmark, "~> 1.4.15"}, {:bbcode_pleroma, "~> 0.2.0"}, {:crypt, diff --git a/mix.lock b/mix.lock index 9a23c8987..8c39d2199 100644 --- a/mix.lock +++ b/mix.lock @@ -125,7 +125,7 @@ "search_parser": {:git, "https://github.com/FloatingGhost/pleroma-contrib-search-parser.git", "08971a81e68686f9ac465cfb6661d51c5e4e1e7f", [ref: "08971a81e68686f9ac465cfb6661d51c5e4e1e7f"]}, "sleeplocks": {:hex, :sleeplocks, "1.1.1", "3d462a0639a6ef36cc75d6038b7393ae537ab394641beb59830a1b8271faeed3", [:rebar3], [], "hexpm", "84ee37aeff4d0d92b290fff986d6a95ac5eedf9b383fadfd1d88e9b84a1c02e1"}, "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.6", "cf344f5692c82d2cd7554f5ec8fd961548d4fd09e7d22f5b62482e5aeaebd4b0", [:make, :mix, :rebar3], [], "hexpm", "bdb0d2471f453c88ff3908e7686f86f9be327d065cc1ec16fa4540197ea04680"}, - "sweet_xml": {:hex, :sweet_xml, "0.6.6", "fc3e91ec5dd7c787b6195757fbcf0abc670cee1e4172687b45183032221b66b8", [:mix], [], "hexpm", "2e1ec458f892ffa81f9f8386e3f35a1af6db7a7a37748a64478f13163a1f3573"}, + "sweet_xml": {:hex, :sweet_xml, "0.7.2", "4729f997286811fabdd8288f8474e0840a76573051062f066c4b597e76f14f9f", [:mix], [], "hexpm", "6894e68a120f454534d99045ea3325f7740ea71260bc315f82e29731d570a6e8"}, "swoosh": {:hex, :swoosh, "1.3.11", "34f79c57f19892b43bd2168de9ff5de478a721a26328ef59567aad4243e7a77b", [:mix], [{:cowboy, "~> 1.1 or ~> 2.4", [hex: :cowboy, repo: "hexpm", optional: true]}, {:finch, "~> 0.6", [hex: :finch, repo: "hexpm", optional: true]}, {:gen_smtp, "~> 0.13 or ~> 1.0", [hex: :gen_smtp, repo: "hexpm", optional: true]}, {:hackney, "~> 1.9", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mail, "~> 0.2", [hex: :mail, repo: "hexpm", optional: true]}, {:mime, "~> 1.1", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_cowboy, ">= 1.0.0", [hex: :plug_cowboy, repo: "hexpm", optional: true]}], "hexpm", "f1e2a048db454f9982b9cf840f75e7399dd48be31ecc2a7dc10012a803b913af"}, "syslog": {:hex, :syslog, "1.1.0", "6419a232bea84f07b56dc575225007ffe34d9fdc91abe6f1b2f254fd71d8efc2", [:rebar3], [], "hexpm", "4c6a41373c7e20587be33ef841d3de6f3beba08519809329ecc4d27b15b659e1"}, "table_rex": {:hex, :table_rex, "3.1.1", "0c67164d1714b5e806d5067c1e96ff098ba7ae79413cc075973e17c38a587caa", [:mix], [], "hexpm", "678a23aba4d670419c23c17790f9dcd635a4a89022040df7d5d772cb21012490"}, diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index 50315e21f..511405624 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -291,6 +291,30 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do assert json_response(conn, 200) == ObjectView.render("object.json", %{object: note}) end + test "does not cache authenticated response", %{conn: conn} do + user = insert(:user) + reader = insert(:user) + + {:ok, post} = + CommonAPI.post(user, %{status: "test @#{reader.nickname}", visibility: "local"}) + + object = Object.normalize(post, fetch: false) + uuid = String.split(object.data["id"], "/") |> List.last() + + assert response = + conn + |> assign(:user, reader) + |> put_req_header("accept", "application/activity+json") + |> get("/objects/#{uuid}") + + json_response(response, 200) + + conn + |> put_req_header("accept", "application/activity+json") + |> get("/objects/#{uuid}") + |> json_response(404) + end + test "it returns 404 for non-public messages", %{conn: conn} do note = insert(:direct_note) uuid = String.split(note.data["id"], "/") |> List.last() diff --git a/test/pleroma/web/plugs/cache_test.exs b/test/pleroma/web/plugs/cache_test.exs index 0ceab6cab..4e729cafb 100644 --- a/test/pleroma/web/plugs/cache_test.exs +++ b/test/pleroma/web/plugs/cache_test.exs @@ -179,4 +179,22 @@ defmodule Pleroma.Web.Plugs.CacheTest do |> send_resp(:im_a_teapot, "🥤") |> sent_resp() end + + test "ignores if skip_cache is assigned" do + assert @miss_resp == + conn(:get, "/") + |> assign(:skip_cache, true) + |> Cache.call(%{query_params: false, ttl: nil}) + |> put_resp_content_type("cofe/hot") + |> send_resp(:ok, "cofe") + |> sent_resp() + + assert @miss_resp == + conn(:get, "/") + |> assign(:skip_cache, true) + |> Cache.call(%{query_params: false, ttl: nil}) + |> put_resp_content_type("cofe/hot") + |> send_resp(:ok, "cofe") + |> sent_resp() + end end