diff --git a/CHANGELOG.md b/CHANGELOG.md index 12f7e8fab9e66d485c1677454a5232297d123986..37df345ede5ad12372fe7f5dab7e9d7f459be8de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] +### Security +- Mastodon API: Fix being able to request enourmous amount of statuses in timelines leading to DoS. Now limited to 40 per request. + ### Removed - **Breaking**: Removed 1.0+ deprecated configurations `Pleroma.Upload, :strip_exif` and `:instance, :dedupe_media` - **Breaking**: OStatus protocol support @@ -56,6 +59,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Admin API: Render whole status in grouped reports - Mastodon API: User timelines will now respect blocks, unless you are getting the user timeline of somebody you blocked (which would be empty otherwise). - Mastodon API: Favoriting / Repeating a post multiple times will now return the identical response every time. Before, executing that action twice would return an error ("already favorited") on the second try. +- Mastodon API: Limit timeline requests to 3 per timeline per 500ms per user/ip by default. </details> ### Added diff --git a/config/config.exs b/config/config.exs index 0dde1fc85a99676b8db4f1709e4c9b2f643baed8..9c4eb70a337911846465157a9cbebfb3d55fc3f7 100644 --- a/config/config.exs +++ b/config/config.exs @@ -599,6 +599,7 @@ config :pleroma, :rate_limit, authentication: {60_000, 15}, + timeline: {500, 3}, search: [{1000, 10}, {1000, 30}], app_account_creation: {1_800_000, 25}, relations_actions: {10_000, 10}, diff --git a/config/description.exs b/config/description.exs index bcb69bc4105ab35999d3afddf543efdfd44148cf..9fdcfcd967883f02dd3772b995247e29e5b5b6be 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2465,6 +2465,12 @@ description: "For the search requests (account & status search etc.)", suggestions: [{1000, 10}, [{10_000, 10}, {10_000, 50}]] }, + %{ + key: :timeline, + type: [:tuple, {:list, :tuple}], + description: "For requests to timelines (each timeline has it's own limiter)", + suggestions: [{1000, 10}, [{10_000, 10}, {10_000, 50}]] + }, %{ key: :app_account_creation, type: [:tuple, {:list, :tuple}], diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index ac55a0b32319fae9c722a73c5bdef31ae83fa9d5..1cffae9773e41cc6aaad389184daa384522f694c 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -343,6 +343,7 @@ Means that: Supported rate limiters: * `:search` - Account/Status search. +* `:timeline` - Timeline requests (each timeline has it's own limiter). * `:app_account_creation` - Account registration from the API. * `:relations_actions` - Following/Unfollowing in general. * `:relation_id_action` - Following/Unfollowing for a specific user. diff --git a/lib/pleroma/pagination.ex b/lib/pleroma/pagination.ex index 4535ca7c5c2b65d3d68b9acf469996ccfec8da51..43fb7babfadcc448f98761fd86659d0bc4634f05 100644 --- a/lib/pleroma/pagination.ex +++ b/lib/pleroma/pagination.ex @@ -13,6 +13,7 @@ defmodule Pleroma.Pagination do alias Pleroma.Repo @default_limit 20 + @max_limit 40 @page_keys ["max_id", "min_id", "limit", "since_id", "order"] def page_keys, do: @page_keys @@ -130,7 +131,11 @@ defp restrict(query, :offset, %{offset: offset}, _table_binding) do end defp restrict(query, :limit, options, _table_binding) do - limit = Map.get(options, :limit, @default_limit) + limit = + case Map.get(options, :limit, @default_limit) do + limit when limit < @max_limit -> limit + _ -> @max_limit + end query |> limit(^limit) diff --git a/lib/pleroma/plugs/rate_limiter/limiter_supervisor.ex b/lib/pleroma/plugs/rate_limiter/limiter_supervisor.ex index 187582edef4c9849f794766d1c14a94d7a3fd287..884268d96249572784d3b0458fa6f7fc43feb28e 100644 --- a/lib/pleroma/plugs/rate_limiter/limiter_supervisor.ex +++ b/lib/pleroma/plugs/rate_limiter/limiter_supervisor.ex @@ -7,8 +7,8 @@ def start_link(init_arg) do DynamicSupervisor.start_link(__MODULE__, init_arg, name: __MODULE__) end - def add_limiter(limiter_name, expiration) do - {:ok, _pid} = + def add_or_return_limiter(limiter_name, expiration) do + result = DynamicSupervisor.start_child( __MODULE__, %{ @@ -28,6 +28,12 @@ def add_limiter(limiter_name, expiration) do ]} } ) + + case result do + {:ok, _pid} = result -> result + {:error, {:already_started, pid}} -> {:ok, pid} + _ -> result + end end @impl true diff --git a/lib/pleroma/plugs/rate_limiter/rate_limiter.ex b/lib/pleroma/plugs/rate_limiter/rate_limiter.ex index 9c362a392c3663b0d542310e04f3b1d5df7931f3..b9cbe9716d5be00ee33dec3ff8b377be644424dd 100644 --- a/lib/pleroma/plugs/rate_limiter/rate_limiter.ex +++ b/lib/pleroma/plugs/rate_limiter/rate_limiter.ex @@ -171,7 +171,7 @@ defp check_rate(action_settings) do {:error, value} {:error, :no_cache} -> - initialize_buckets(action_settings) + initialize_buckets!(action_settings) check_rate(action_settings) end end @@ -250,11 +250,16 @@ defp attach_selected_params(input, %{conn_params: conn_params, opts: plug_opts}) |> String.replace_leading(":", "") end - defp initialize_buckets(%{name: _name, limits: nil}), do: :ok + defp initialize_buckets!(%{name: _name, limits: nil}), do: :ok - defp initialize_buckets(%{name: name, limits: limits}) do - LimiterSupervisor.add_limiter(anon_bucket_name(name), get_scale(:anon, limits)) - LimiterSupervisor.add_limiter(user_bucket_name(name), get_scale(:user, limits)) + defp initialize_buckets!(%{name: name, limits: limits}) do + {:ok, _pid} = + LimiterSupervisor.add_or_return_limiter(anon_bucket_name(name), get_scale(:anon, limits)) + + {:ok, _pid} = + LimiterSupervisor.add_or_return_limiter(user_bucket_name(name), get_scale(:user, limits)) + + :ok end defp attach_identity(base, %{mode: :user, conn_info: conn_info}), diff --git a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex index 29964a1d41a46495303502f5f40d85c6519baae1..a3110c722a80e33c4da2ed152d5e4c722a3b4ddf 100644 --- a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex @@ -10,9 +10,20 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do alias Pleroma.Pagination alias Pleroma.Plugs.OAuthScopesPlug + alias Pleroma.Plugs.RateLimiter alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub + # TODO: Replace with a macro when there is a Phoenix release with + # https://github.com/phoenixframework/phoenix/commit/2e8c63c01fec4dde5467dbbbf9705ff9e780735e + # in it + + plug(RateLimiter, [name: :timeline, bucket_name: :direct_timeline] when action == :direct) + plug(RateLimiter, [name: :timeline, bucket_name: :public_timeline] when action == :public) + plug(RateLimiter, [name: :timeline, bucket_name: :home_timeline] when action == :home) + plug(RateLimiter, [name: :timeline, bucket_name: :hashtag_timeline] when action == :hashtag) + plug(RateLimiter, [name: :timeline, bucket_name: :list_timeline] when action == :list) + plug(OAuthScopesPlug, %{scopes: ["read:statuses"]} when action in [:home, :direct]) plug(OAuthScopesPlug, %{scopes: ["read:lists"]} when action == :list) diff --git a/test/plugs/rate_limiter_test.exs b/test/plugs/rate_limiter_test.exs index 104d67611bff9028c8ce191fc027b90694c6d819..8cdc8d1a2de0bdaca4848b4bc86b0809436fdede 100644 --- a/test/plugs/rate_limiter_test.exs +++ b/test/plugs/rate_limiter_test.exs @@ -242,4 +242,35 @@ test "different users are counted independently" do refute conn_2.halted end end + + test "doesn't crash due to a race condition when multiple requests are made at the same time and the bucket is not yet initialized" do + limiter_name = :test_race_condition + Pleroma.Config.put([:rate_limit, limiter_name], {1000, 5}) + Pleroma.Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8}) + + opts = RateLimiter.init(name: limiter_name) + + conn = conn(:get, "/") + conn_2 = conn(:get, "/") + + %Task{pid: pid1} = + task1 = + Task.async(fn -> + receive do + :process2_up -> + RateLimiter.call(conn, opts) + end + end) + + task2 = + Task.async(fn -> + send(pid1, :process2_up) + RateLimiter.call(conn_2, opts) + end) + + Task.await(task1) + Task.await(task2) + + refute {:err, :not_found} == RateLimiter.inspect_bucket(conn, limiter_name, opts) + end end