From 5f71354f52eda5fed755aa220d024d2d4a398852 Mon Sep 17 00:00:00 2001 From: Edward Oliveira Date: Mon, 8 Jun 2026 15:42:13 -0300 Subject: [PATCH] Fix: same-origin bypass let blocked IPs through on /api/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _handle_api checked _is_same_origin() before any of the IP-prefix, global (rl:ip::blocked), and API-specific (rl:api:ip::blocked) block keys — short-circuiting straight to get_response() on a match. Since Origin/Referer are entirely client-controlled and trivially spoofable, any caller could defeat every /api/ block and counter (including an operator-set global block) by simply sending Origin: https://. Reorder the checks so IP-prefix/global/API block lookups always run first; the same-origin bypass now only exempts legitimate same-origin polling from quota and per-minute rate-limit accounting, never from an active block. Co-Authored-By: Claude Sonnet 4.6 --- sapl/middleware/ratelimit.py | 36 +++++++++------ sapl/middleware/test_ratelimiter.py | 70 +++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 13 deletions(-) diff --git a/sapl/middleware/ratelimit.py b/sapl/middleware/ratelimit.py index 8c2489d57..0aafbfe3d 100644 --- a/sapl/middleware/ratelimit.py +++ b/sapl/middleware/ratelimit.py @@ -6,14 +6,19 @@ Decision flow (per request): -1. IP in rl:ip_prefix:blocked (prefix match)? → 429 (universal — applies to authenticated users too, like the UA bot checks) /api/ paths — handled by _handle_api: - 0a. OPTIONS? → pass (CORS preflight must never be blocked) - 0b. Same-origin? → pass (SAPL's own browser polling) - 0c. rl:ip::blocked? → 429 (global block also covers /api/) + 0a. OPTIONS? → pass (CORS preflight must never be blocked) + 0b. rl:ip_prefix:blocked? → 429 (prefix match; see -1 above) + 0c. rl:ip::blocked? → 429 (global block also covers /api/) 0d. rl:api:ip::blocked? → 429 (API-only block) - 0e. Daily/weekly quota exceeded? → 429 - 0f. Anon + API threshold exceeded? → SET rl:api:ip::blocked, 429 + Block checks 0b-0d always run before the same-origin check below — + Origin/Referer are client-controlled and trivially spoofable, so they + must never be able to override an already-made block decision. + 0e. Same-origin? → pass, skipping 0f-0g only (SAPL's own polling + is exempt from quota/rate-limit accounting, never from active blocks) + 0f. Daily/weekly quota exceeded? → 429 + 0g. Anon + API threshold exceeded? → SET rl:api:ip::blocked, 429 (never writes rl:ip::blocked) - 0g. Auth: falls through to _evaluate (per-user counter) + 0h. Auth: falls through to _evaluate (per-user counter) Non-/api/ paths: 1. Known bot UA? → 429 (Python list — substring match) 1b. Redis UA deny list? → 429 (runtime SET — token hash match, refreshed every 60 s) @@ -448,13 +453,12 @@ class RateLimitMiddleware: if request.method == 'OPTIONS': return self.get_response(request) - # 2. Same-origin (SAPL's own polling) — no counter, no block - if self.api_same_origin_bypass and _is_same_origin(request): - return self.get_response(request) - ip = get_client_ip(request) - # 3a. IP-prefix block — operator-curated deny list, applies to everyone + # 2. IP-prefix block — operator-curated deny list, applies to everyone. + # Block checks (2-4) must run before the same-origin bypass below: + # Origin/Referer are client-controlled and trivially spoofable, so + # they must never be able to override an already-made block decision. if self._is_ip_prefix_blocked(ip): logger.warning( 'api_rate_limit_block reason=ip_prefix_blocked ip=%s path=%s user_agent=%s', @@ -481,7 +485,13 @@ class RateLimitMiddleware: self._inc_block_metric('api_ip_blocked') return self._api_block_response('api_ip_blocked') - # 5. Daily/weekly quota (existing logic, preserved) + # 5. Same-origin (SAPL's own polling) — exempt from quota/rate-limit + # accounting only (steps 6-7 below). Must come after the block + # checks above, never before them. + if self.api_same_origin_bypass and _is_same_origin(request): + return self.get_response(request) + + # 6. Daily/weekly quota (existing logic, preserved) exceeded = self._check_api_quota(request) if exceeded: logger.warning( @@ -495,7 +505,7 @@ class RateLimitMiddleware: response['X-RateLimit-Reason'] = f'quota_{exceeded}' return response - # 6. Per-minute rate limit — 60/min for all callers (anon and auth). + # 7. Per-minute rate limit — 60/min for all callers (anon and auth). # Auth is not exempt: authenticating must not bypass this cap. # Writes rl:api:ip::blocked only — never rl:ip::blocked. if self.api_rate_limit_enabled: diff --git a/sapl/middleware/test_ratelimiter.py b/sapl/middleware/test_ratelimiter.py index 9b4c6b5cb..3fb585a7a 100644 --- a/sapl/middleware/test_ratelimiter.py +++ b/sapl/middleware/test_ratelimiter.py @@ -647,6 +647,76 @@ def test_api_malicious_origin_is_not_same_origin(): mw._incr_with_ttl.assert_called_once() +# Same-origin headers a spoofing client can trivially forge — Origin/Referer +# carry no authentication, so they must never override an active block. +_SAME_ORIGIN_META = { + 'SERVER_NAME': 'sapl.example.com', + 'SERVER_PORT': '80', + 'HTTP_HOST': 'sapl.example.com', + 'HTTP_ORIGIN': 'https://sapl.example.com', +} + + +def test_api_same_origin_does_not_bypass_global_ip_block(): + mw, mock_cache = _make_middleware() + ip = '1.2.3.4' + mock_cache.get.side_effect = lambda key: 1 if key == RL_IP_BLOCKED.format(ip=ip) else None + mw._check_api_quota = MagicMock(return_value=None) + mw._incr_with_ttl = MagicMock() + request = _api_req(ip=ip, extra_meta=_SAME_ORIGIN_META) + + response = mw(request) + + mw.get_response.assert_not_called() + assert response.status_code == 429 + assert response['X-RateLimit-Reason'] == 'global_ip_blocked' + + +def test_api_same_origin_does_not_bypass_api_ip_block(): + mw, mock_cache = _make_middleware() + ip = '1.2.3.4' + blocked_key = RL_API_IP_BLOCKED.format(ns=_NAMESPACE, ip=ip) + mock_cache.get.side_effect = lambda key: 1 if key == blocked_key else None + mw._check_api_quota = MagicMock(return_value=None) + mw._incr_with_ttl = MagicMock() + request = _api_req(ip=ip, extra_meta=_SAME_ORIGIN_META) + + response = mw(request) + + mw.get_response.assert_not_called() + assert response.status_code == 429 + assert response['X-RateLimit-Reason'] == 'api_ip_blocked' + + +def test_api_same_origin_does_not_bypass_ip_prefix_block(_seed_prefix_blocklist): + mw, _ = _make_middleware() + _seed_prefix_blocklist(['1.2.3']) + mw._check_api_quota = MagicMock(return_value=None) + mw._incr_with_ttl = MagicMock() + request = _api_req(ip='1.2.3.4', extra_meta=_SAME_ORIGIN_META) + + response = mw(request) + + mw.get_response.assert_not_called() + assert response.status_code == 429 + assert response['X-RateLimit-Reason'] == 'ip_prefix_blocked' + + +def test_api_same_origin_still_skips_quota_and_rate_limit_when_not_blocked(): + """Legitimate same-origin polling keeps its original exemption from accounting.""" + mw, _ = _make_middleware() + mw._check_api_quota = MagicMock(return_value=None) + mw._incr_with_ttl = MagicMock() + request = _api_req(extra_meta=_SAME_ORIGIN_META) + + response = mw(request) + + mw.get_response.assert_called_once_with(request) + mw._check_api_quota.assert_not_called() + mw._incr_with_ttl.assert_not_called() + assert response.status_code == 200 + + # --------------------------------------------------------------------------- # _handle_api — Check 3a: IP-prefix block (operator-curated deny list) # ---------------------------------------------------------------------------