Skip to content

Commit 77d680e

Browse files
committed
Add separate player creation rate limiting (addresses @sydseter feedback)
-->Added :player_creation action to RateLimiter with separate limits --> Default: 20 players per IP per hour (separate from game creation) --> Applied rate limiting to PlayerLive.FormComponent --> Added comprehensive tests for player creation limits --> Updated documentation with implementation details Fixes #1877
1 parent 49c614a commit 77d680e

File tree

4 files changed

+206
-14
lines changed

4 files changed

+206
-14
lines changed

RATE_LIMITING_IMPLEMENTATION.md

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
# Rate Limiting Implementation for Player Creation
2+
3+
## Overview
4+
This implementation addresses GitHub Issue #1877 by adding IP-based rate limiting for player creation, separate from the existing game creation rate limit, to protect against CAPEC 212 (Functionality Misuse) attacks.
5+
6+
## Changes Made
7+
8+
### 1. Rate Limiter Module (`lib/copi/rate_limiter.ex`)
9+
10+
**Added player_creation action:**
11+
- Updated `check_rate/2` to accept `:player_creation` action
12+
- Updated `record_action/2` to accept `:player_creation` action
13+
- Added player creation configuration with default limits:
14+
- Maximum players per IP: 20
15+
- Time window: 3600 seconds (1 hour)
16+
17+
**Configuration:**
18+
```elixir
19+
player_creation: %{
20+
max_requests: get_env(:max_players_per_ip, 20),
21+
window_seconds: get_env(:player_creation_window_seconds, 3600)
22+
}
23+
```
24+
25+
### 2. Player Form Component (`lib/copi_web/live/player_live/form_component.ex`)
26+
27+
**Added rate limiting to player creation:**
28+
- Added `get_connect_ip/1` helper function to extract IP address from socket
29+
- Modified `save_player/3` for `:new` action to:
30+
1. Get the connecting IP address
31+
2. Check rate limit before creating player
32+
3. Only create player if rate limit is not exceeded
33+
4. Record the action after successful creation
34+
5. Display user-friendly error message if rate limited
35+
36+
**Error message shown to users:**
37+
```
38+
"Rate limit exceeded. Too many players created from your IP address.
39+
Please try again in X seconds. This limit helps ensure service
40+
availability for all users."
41+
```
42+
43+
### 3. Tests (`test/copi/rate_limiter_test.exs`)
44+
45+
**Added comprehensive test coverage:**
46+
- Test that player creation is allowed under the limit
47+
- Test that player creation is blocked when limit is exceeded
48+
- Test that player creation limit is separate from game creation limit
49+
- Updated configuration test to verify player_creation config exists
50+
51+
## Key Features
52+
53+
### Separate Limits
54+
The player creation rate limit is maintained **separately** from the game creation rate limit. This means:
55+
- An IP that has exhausted its game creation quota can still create players
56+
- An IP that has exhausted its player creation quota can still create games
57+
- Each limit is tracked independently in the GenServer state
58+
59+
### Configurable Limits
60+
The limits can be configured via application environment:
61+
```elixir
62+
config :copi, Copi.RateLimiter,
63+
max_players_per_ip: 20,
64+
player_creation_window_seconds: 3600
65+
```
66+
67+
### User-Friendly Error Handling
68+
When rate limited, users receive:
69+
- Clear explanation of why they were blocked
70+
- Time until they can try again (retry_after in seconds)
71+
- Explanation that this protects service availability
72+
73+
## Security Benefits
74+
75+
1. **CAPEC 212 Mitigation**: Prevents functionality misuse by limiting the rate of player creation from a single IP
76+
2. **DoS Protection**: Helps maintain service availability under attack
77+
3. **Resource Conservation**: Prevents database and system resource exhaustion
78+
4. **Granular Control**: Separate limits allow fine-tuned protection for different actions
79+
80+
## Default Limits Summary
81+
82+
| Action | Max Requests | Time Window |
83+
|--------|--------------|-------------|
84+
| Game Creation | 10 | 1 hour |
85+
| Player Creation | 20 | 1 hour |
86+
| Connection | 50 | 5 minutes |
87+
88+
## Testing
89+
90+
Run the test suite:
91+
```bash
92+
mix test test/copi/rate_limiter_test.exs
93+
```
94+
95+
All tests should pass, including the new player creation rate limiting tests.
96+
97+
## Future Enhancements
98+
99+
As mentioned in the issue, if this is still insufficient, the next step would be:
100+
- Implement authentication
101+
- Associate rate limits with user accounts in addition to IP addresses
102+
- Track browser fingerprints along with IP addresses
103+
104+
---
105+
106+
**Issue Reference:** OWASP/cornucopia#1877
107+
**Related Security Control:** CAPEC-212 (Functionality Misuse)
108+
**Implemented by:** @immortal71

copi.owasp.org/lib/copi/rate_limiter.ex

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ defmodule Copi.RateLimiter do
22
@moduledoc """
33
Rate limiter to prevent abuse by limiting requests per IP address.
44
5-
This module implements rate limiting for game creation and user connections
5+
This module implements rate limiting for game creation, player creation, and user connections
66
to protect against CAPEC 212 (Functionality Misuse) attacks.
77
"""
88

@@ -25,14 +25,14 @@ defmodule Copi.RateLimiter do
2525
2626
Returns `{:ok, remaining}` if allowed, `{:error, :rate_limited, retry_after}` if blocked.
2727
"""
28-
def check_rate(ip_address, action) when action in [:game_creation, :connection] do
28+
def check_rate(ip_address, action) when action in [:game_creation, :player_creation, :connection] do
2929
GenServer.call(__MODULE__, {:check_rate, ip_address, action})
3030
end
3131

3232
@doc """
3333
Records a successful action for rate limiting tracking.
3434
"""
35-
def record_action(ip_address, action) when action in [:game_creation, :connection] do
35+
def record_action(ip_address, action) when action in [:game_creation, :player_creation, :connection] do
3636
GenServer.cast(__MODULE__, {:record_action, ip_address, action})
3737
end
3838

@@ -62,6 +62,10 @@ defmodule Copi.RateLimiter do
6262
max_requests: get_env(:max_games_per_ip, 10),
6363
window_seconds: get_env(:game_creation_window_seconds, 3600)
6464
},
65+
player_creation: %{
66+
max_requests: get_env(:max_players_per_ip, 20),
67+
window_seconds: get_env(:player_creation_window_seconds, 3600)
68+
},
6569
connection: %{
6670
max_requests: get_env(:max_connections_per_ip, 50),
6771
window_seconds: get_env(:connection_window_seconds, 300)

copi.owasp.org/lib/copi_web/live/player_live/form_component.ex

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -74,23 +74,52 @@ defmodule CopiWeb.PlayerLive.FormComponent do
7474
end
7575

7676
defp save_player(socket, :new, player_params) do
77-
case Cornucopia.create_player(player_params) do
78-
{:ok, player} ->
79-
80-
{:ok, updated_game} = Cornucopia.Game.find(socket.assigns.player.game_id)
81-
CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)
82-
77+
# Get the IP address for rate limiting
78+
ip_address = get_connect_ip(socket)
79+
80+
# Check rate limit before creating player
81+
case Copi.RateLimiter.check_rate(ip_address, :player_creation) do
82+
{:ok, _remaining} ->
83+
case Cornucopia.create_player(player_params) do
84+
{:ok, player} ->
85+
# Record the action after successful creation
86+
Copi.RateLimiter.record_action(ip_address, :player_creation)
87+
88+
{:ok, updated_game} = Cornucopia.Game.find(socket.assigns.player.game_id)
89+
CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)
90+
91+
{:noreply,
92+
socket
93+
|> assign(:game, updated_game)
94+
|> push_navigate(to: ~p"/games/#{player.game_id}/players/#{player.id}")}
95+
96+
{:error, %Ecto.Changeset{} = changeset} ->
97+
{:noreply, assign_form(socket, changeset)}
98+
end
99+
100+
{:error, :rate_limited, retry_after} ->
83101
{:noreply,
84102
socket
85-
|> assign(:game, updated_game)
86-
|> push_navigate(to: ~p"/games/#{player.game_id}/players/#{player.id}")}
87-
88-
{:error, %Ecto.Changeset{} = changeset} ->
89-
{:noreply, assign_form(socket, changeset)}
103+
|> put_flash(
104+
:error,
105+
"Rate limit exceeded. Too many players created from your IP address. " <>
106+
"Please try again in #{retry_after} seconds. " <>
107+
"This limit helps ensure service availability for all users."
108+
)
109+
|> assign_form(socket.assigns.form.source)}
90110
end
91111
end
92112

93113
def topic(game_id) do
94114
"game:#{game_id}"
95115
end
116+
117+
defp get_connect_ip(socket) do
118+
case get_connect_info(socket, :peer_data) do
119+
%{address: {a, b, c, d}} -> "#{a}.#{b}.#{c}.#{d}"
120+
%{address: {a, b, c, d, e, f, g, h}} ->
121+
"#{Integer.to_string(a, 16)}:#{Integer.to_string(b, 16)}:#{Integer.to_string(c, 16)}:#{Integer.to_string(d, 16)}:#{Integer.to_string(e, 16)}:#{Integer.to_string(f, 16)}:#{Integer.to_string(g, 16)}:#{Integer.to_string(h, 16)}"
122+
_ -> "unknown"
123+
end
124+
end
96125
end

copi.owasp.org/test/copi/rate_limiter_test.exs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,53 @@ defmodule Copi.RateLimiterTest do
9696
end
9797
end
9898

99+
describe "player creation rate limiting" do
100+
test "allows player creation under the limit" do
101+
ip = "192.168.2.1"
102+
103+
assert {:ok, remaining} = RateLimiter.check_rate(ip, :player_creation)
104+
assert remaining >= 0
105+
106+
RateLimiter.record_action(ip, :player_creation)
107+
108+
assert {:ok, _remaining} = RateLimiter.check_rate(ip, :player_creation)
109+
end
110+
111+
test "blocks player creation over the limit" do
112+
ip = "192.168.2.2"
113+
config = RateLimiter.get_config()
114+
max_players = config.player_creation.max_requests
115+
116+
# Make max_requests number of player creations
117+
for _i <- 1..max_players do
118+
assert {:ok, _remaining} = RateLimiter.check_rate(ip, :player_creation)
119+
RateLimiter.record_action(ip, :player_creation)
120+
end
121+
122+
# Next request should be blocked
123+
assert {:error, :rate_limited, retry_after} = RateLimiter.check_rate(ip, :player_creation)
124+
assert retry_after > 0
125+
end
126+
127+
test "player creation limit is separate from game creation limit" do
128+
ip = "192.168.2.3"
129+
config = RateLimiter.get_config()
130+
max_games = config.game_creation.max_requests
131+
132+
# Exhaust game creation limit
133+
for _i <- 1..max_games do
134+
RateLimiter.check_rate(ip, :game_creation)
135+
RateLimiter.record_action(ip, :game_creation)
136+
end
137+
138+
# Game creation should be blocked
139+
assert {:error, :rate_limited, _} = RateLimiter.check_rate(ip, :game_creation)
140+
141+
# Player creation should still be allowed (separate limit)
142+
assert {:ok, _remaining} = RateLimiter.check_rate(ip, :player_creation)
143+
end
144+
end
145+
99146
describe "rate limit window expiration" do
100147
test "allows requests after window expires" do
101148
ip = "192.168.100.1"
@@ -118,11 +165,15 @@ defmodule Copi.RateLimiterTest do
118165

119166
assert is_map(config)
120167
assert Map.has_key?(config, :game_creation)
168+
assert Map.has_key?(config, :player_creation)
121169
assert Map.has_key?(config, :connection)
122170

123171
assert config.game_creation.max_requests > 0
124172
assert config.game_creation.window_seconds > 0
125173

174+
assert config.player_creation.max_requests > 0
175+
assert config.player_creation.window_seconds > 0
176+
126177
assert config.connection.max_requests > 0
127178
assert config.connection.window_seconds > 0
128179
end

0 commit comments

Comments
 (0)