[Security] Fix ReDoS in validation error path sanitizer - #52120
[Security] Fix ReDoS in validation error path sanitizer#52120jperezdealgaba wants to merge 1 commit into
Conversation
The generic file-path regex in sanitize_message used a pattern with nested repetition that caused catastrophic backtracking on long slash-delimited strings without a filename extension. A 100KB malformed request field could keep the sanitizer busy for 3+ seconds, allowing a remote client to degrade API worker availability. Fix: skip the generic path regex entirely when the message contains no dot character (the required extension anchor), and use an atomic group (?>...) to prevent backtracking within the repeated slash-segments on messages that do contain dots. Both changes ensure near-linear-time sanitization regardless of input shape. Addresses GHSA-f2g9-pmwr-xwc7 (CWE-1333). Signed-off-by: Juan Pérez de Algaba <jperezde@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: jperezde <jperezde@redhat.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
github-actions
Bot
added
the
ready
|
/ci run |
|
✅ Triggered Buildkite CI #83696 for commit |
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
| if "." in message: | ||
| message = re.sub(r"(?>/[\w\-]+)+/[\w\-]+\.\w+", "<path>", message) |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
The atomic group (?>...) prevents exponential backtracking but still exhibits O(n²) behavior: re.sub retries the pattern at every /-position, each time the atomic group scans the remaining input. Input like "/a" * 5000 + " error.txt" (dot bypasses the pre-check) takes ~9 seconds; at 50k segments it would block a worker for minutes. The original advisory PoC shape is only partially mitigated.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Add a message length guard to the if condition on line 319 to bound the O(n²) behavior. The re.sub retries the atomic-group pattern at every /-position, each scanning forward through the remaining input, creating quadratic time complexity. By capping the message length for this specific regex (e.g., len(message) <= 2048), the worst-case time is bounded to ~30ms. Legitimate error messages with file paths are well under 2KB, and the preceding known-root-directory regex on lines 316-318 already handles common paths (/home/..., /usr/..., etc.) without this quadratic issue. Alternatively, for a more comprehensive fix, consider truncating the message at the top of sanitize_message() (e.g., message = message[:4096]) to protect all regexes in the function, or replace the regex with a linear-time token-based approach.
⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.
| if "." in message: | |
| message = re.sub(r"(?>/[\w\-]+)+/[\w\-]+\.\w+", "<path>", message) | |
| if "." in message and len(message) <= 2048: | |
| message = re.sub(r"(?>/[\w\-]+)+/[\w\-]+\.\w+", "<path>", message) |
|
This pull request has merge conflicts that must be resolved before it can be |
2 participants
Summary
sanitize_message()where a 100KB slash-delimited input ("/a" * 50000) could keep an API worker busy for 3+ seconds per rejected request.(?>...)to prevent backtracking within repeated slash-segments for messages that do contain dots.Test plan
pytest tests/entrypoints/serve/utils/test_api_utils.py -v— 22 tests pass (4 new ReDoS regression tests)pytest tests/entrypoints/serve/utils/test_error_sanitization.py -v— 9 tests passpre-commit run --files <changed files>— all hooks pass"/a" * 50000): completes in <0.1ms after fix (was 3+ second hang)/app/server.py,/usr/lib/...etc.)Made with Cursor