fix(security): reject universal TRUSTED_PROXY_IPS catch-all CIDRs - #127
Merged
Conversation
TRUSTED_PROXY_IPS=0.0.0.0/0 or ::/0 makes every direct TCP peer look like a trusted proxy, so client_ip_for_rate_limit() buckets by spoofed X-Forwarded-For values and the /login rate limit can be bypassed. Co-authored-by: Alexander Wagner <[email protected]>
Co-authored-by: Alexander Wagner <[email protected]>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bug-scan pass on
config.pyfound thatTRUSTED_PROXY_IPS=0.0.0.0/0or::/0was accepted at startup. These catch-all CIDRs make every direct TCP peer look like a trusted proxy, soclient_ip_for_rate_limit()buckets by spoofedX-Forwarded-Forvalues and the/loginrate limit can be bypassed.Bug and impact
TRUSTED_PROXY_COUNT=1withTRUSTED_PROXY_IPS=0.0.0.0/0(or::/0). Attacker connects directly to port 8000 with a forgedX-Forwarded-Forheader and rotates spoofed IPs to bypass the 5/min login cap.Fix
Reject universal (
/0) networks in_parse_trusted_proxy_networks()with a clear startup error. Added parametrized regression tests for0.0.0.0/0and::/0.Validation
CI should run
pytest(non-integration) including new tests intests/test_proxy_rate_limit_security.py.Note
Medium Risk
Touches trusted-proxy and rate-limit IP selection, which is security-sensitive, but the change is a fail-closed config check with tests and does not alter the happy-path trust logic.
Overview
Startup now rejects
TRUSTED_PROXY_IPSvalues that are universal CIDRs (0.0.0.0/0,::/0). Those ranges made every direct client look like a trusted proxy, so spoofedX-Forwarded-Forcould bypass per-IP/loginrate limits._parse_trusted_proxy_networks()fails closed with a clear config error. Regression tests cover both IPv4 and IPv6 catch-alls.Reviewed by Cursor Bugbot for commit 9139371. Bugbot is set up for automated code reviews on this repo. Configure here.