fix: block TRACE/TRACK methods - #1172
Open
marlonkeating wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the Django MaliciousRequestsMiddleware to mitigate a TRACE/TRACK-based volumetric DDoS by blocking those HTTP methods unconditionally and ensuring method blocking happens before URL/IP checks.
Changes:
- Added unconditional blocking for
TRACEandTRACKvia_ALWAYS_BLOCKED_METHODSand a newcheck_request_method()guard. - Adjusted middleware execution flow so method blocking short-circuits before URL/FWD pattern checks, and removed
MiddlewareNotUsedso the middleware remains active. - Added a new test module covering always-blocked methods plus existing URL/FWD pattern behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| common/helpers/malicious_requests.py | Adds unconditional TRACE/TRACK blocking and keeps middleware active even without env-configured patterns. |
| common/tests/test_malicious_requests_middleware.py | Adds tests for method blocking and existing URL/FWD filtering behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
13
to
18
| if settings.MALICIOUS_URL_PATTERNS is not None: | ||
| url_patterns = settings.MALICIOUS_URL_PATTERNS.split(',') | ||
| self.malicious_url_patterns = list(map(lambda pattern: re.compile(pattern, re.IGNORECASE), url_patterns)) | ||
| used = True | ||
| if settings.MALICIOUS_FWD_PATTERNS is not None: | ||
| fwd_patterns = settings.MALICIOUS_FWD_PATTERNS.split(',') | ||
| self.malicious_fwd_patterns = list(map(lambda pattern: re.compile(pattern, re.IGNORECASE), fwd_patterns)) |
Comment on lines
45
to
46
| hasattr(self, 'malicious_url_patterns') and self.check_request_url(request) | ||
| hasattr(self, 'malicious_fwd_patterns') and self.check_request_fwd(request) |
Comment on lines
+99
to
+105
| def test_clean_fwd_is_allowed(self): | ||
| mw = _middleware() | ||
| mw(_make_request(fwd='203.0.113.5')) | ||
|
|
||
| def test_remote_addr_used_when_no_fwd_header(self): | ||
| mw = _middleware() | ||
| mw(_make_request()) # REMOTE_ADDR is 127.0.0.1, should not match ^10\. |
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
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
democracylab.org experienced a volumetric DDoS attack using HTTP TRACE requests with randomised cache-busting query parameters. The attack saturated Heroku's dyno request backlog, causing H11 errors and 503 responses for legitimate users.
Code fix
File:
common/helpers/malicious_requests.py_ALWAYS_BLOCKED_METHODS = frozenset({'TRACE', 'TRACK'})— blocked unconditionally, no env var needed.check_request_method()is now the first check in__call__, short-circuiting before URL/IP checks.MiddlewareNotUsedraise: the middleware is always active for method blocking even when neither pattern env var is set.Details
Sample log line
Impact
Attack mechanics
method=TRACELt4ZGViwIr=RiPKDIuy8N(randomised query param)fwd=""(empty X-Forwarded-For)X-Forwarded-Forfor all inbound traffic. An empty value suggests the attacker reached the app through an atypical path, or stripped the header. Makes IP-based blocking unreliable for this incident.Background: TRACE and TRACK
TRACE is defined in RFC 9110 §9.3.8 (the current HTTP semantics standard). It is a diagnostic method — the server echoes back the received request so the client can inspect what intermediaries modified in transit. No browser or legitimate API client sends TRACE in production; its only real-world use is manual debugging with
curlor similar tools.Cross-Site Tracing (XST): Described by Jeremiah Grossman in 2003 and documented in OWASP's XST article. Because TRACE echoes all request headers — including
HttpOnlycookies andAuthorizationheaders — back in the response body, a malicious script can read that body and exfiltrate credentials thatHttpOnlywas specifically designed to keep out of JS reach. Modern browsers now block cross-origin TRACE requests, but the method remains a defense-in-depth concern.TRACK is a Microsoft IIS-specific, non-standard variant of TRACE — not defined in any RFC. It carries the same XST risks with no legitimate use case on a Django application.
OWASP's HTTP Methods testing guide recommends disabling both methods on all production servers.
In this incident TRACE was not used for XST but as a flood vector: it is cheap to send, forces the server to process and echo the request body, and — combined with cache-busting query params — bypasses all upstream caching so every request reaches a dyno.
Why existing defences didn't catch it
AnonRateThrottle,UserRateThrottle) only wraps DRF views./user/<id>is a standard Django view, so throttling never fired.MaliciousRequestsMiddlewarefiltered by URL pattern and forwarded-for IP, but had no HTTP method check. It also raisedMiddlewareNotUsedwhen neither env var was set, removing itself from the stack entirely.