commit: 438394d40447bdfb590ff206ad80907294da0e65
parent: 19e559fe5130f66a967732a40ccea1ac39e85eb8
Author: rinpatch <rinpatch@sdf.org>
Date: Sat, 29 Feb 2020 23:08:14 +0000
Merge branch 'fix/easy-timeline-dos' into 'develop'
Cap the number of requested statuses in timelines to 40 and rate limit them
See merge request pleroma/pleroma!2253
Diffstat:
9 files changed, 78 insertions(+), 8 deletions(-)
diff --git 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
@@ -599,6 +599,7 @@ config :http_signatures,
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
@@ -2466,6 +2466,12 @@ config :pleroma, :config_description, [
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}],
description: "For registering user accounts from the same IP address",
diff --git 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
@@ -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 @@ defmodule Pleroma.Pagination 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
@@ -7,8 +7,8 @@ defmodule Pleroma.Plugs.RateLimiter.LimiterSupervisor 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 @@ defmodule Pleroma.Plugs.RateLimiter.LimiterSupervisor 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
@@ -171,7 +171,7 @@ defmodule Pleroma.Plugs.RateLimiter do
{:error, value}
{:error, :no_cache} ->
- initialize_buckets(action_settings)
+ initialize_buckets!(action_settings)
check_rate(action_settings)
end
end
@@ -250,11 +250,16 @@ defmodule Pleroma.Plugs.RateLimiter do
|> 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
@@ -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
@@ -242,4 +242,35 @@ defmodule Pleroma.Plugs.RateLimiterTest 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
