Skip to content

Conversation

@kami922
Copy link
Contributor

@kami922 kami922 commented Jan 3, 2026

Implements comprehensive protection against Regular Expression Denial of Service (ReDoS) attacks in the username filtering system.

Problem

The _check_username_list_regex() function compiled user-provided regex patterns without validation, allowing malicious patterns like (a+)+ to cause exponential-time execution and CPU exhaustion. This could be exploited via Config.ignored_users, Config.enabled_users, or Config.vip_users fields to create a Denial of Service condition.

Solution

Added multi-layer validation in new _is_safe_regex() function:

  1. Length validation: Reject patterns > 100 characters
  2. Complexity check: Reject patterns with > 50 special regex chars
  3. Catastrophic backtracking detection: Identify nested quantifiers
    • Patterns like (a+)+, (a*), (a|ab) are detected and rejected
  4. Syntax validation: Ensure valid regex syntax before compilation

Updated _check_username_list_regex() to validate all patterns before compilation. Unsafe patterns are logged and skipped gracefully.

Testing

  • Added 15 comprehensive tests in TestReDoSProtection class
  • All 35 tests pass (15 new + 20 existing)
  • Tests cover: safe patterns, dangerous patterns, edge cases, performance, and full integration scenarios

Security Impact

  • Prevents ReDoS attacks that could hang the application
  • Maintains backward compatibility with safe regex patterns
  • Minimal performance overhead (< 1ms per pattern validation)
  • Comprehensive logging for security monitoring

Files Changed

  • buffalogs/impossible_travel/modules/alert_filter.py: Added validation
  • buffalogs/impossible_travel/tests/detection/test_alert_filter.py: Added tests
  • REDOS_FIX_DOCUMENTATION.md: Complete technical documentation

Implements comprehensive protection against Regular Expression Denial
of Service (ReDoS) attacks in the username filtering system.

## Problem
The _check_username_list_regex() function compiled user-provided regex
patterns without validation, allowing malicious patterns like (a+)+ to
cause exponential-time execution and CPU exhaustion. This could be
exploited via Config.ignored_users, Config.enabled_users, or
Config.vip_users fields to create a Denial of Service condition.

## Solution
Added multi-layer validation in new _is_safe_regex() function:

1. Length validation: Reject patterns > 100 characters
2. Complexity check: Reject patterns with > 50 special regex chars
3. Catastrophic backtracking detection: Identify nested quantifiers
   - Patterns like (a+)+, (a*)*, (a|ab)* are detected and rejected
4. Syntax validation: Ensure valid regex syntax before compilation

Updated _check_username_list_regex() to validate all patterns before
compilation. Unsafe patterns are logged and skipped gracefully.

## Testing
- Added 15 comprehensive tests in TestReDoSProtection class
- All 35 tests pass (15 new + 20 existing)
- Tests cover: safe patterns, dangerous patterns, edge cases,
  performance, and full integration scenarios

## Security Impact
- Prevents ReDoS attacks that could hang the application
- Maintains backward compatibility with safe regex patterns
- Minimal performance overhead (< 1ms per pattern validation)
- Comprehensive logging for security monitoring

## Files Changed
- buffalogs/impossible_travel/modules/alert_filter.py: Added validation
- buffalogs/impossible_travel/tests/detection/test_alert_filter.py: Added tests
- REDOS_FIX_DOCUMENTATION.md: Complete technical documentation
@Lorygold
Copy link
Collaborator

Lorygold commented Jan 3, 2026

black linter failed in the CI

@Lorygold Lorygold linked an issue Jan 3, 2026 that may be closed by this pull request
Fix CI linting failures by applying Black formatting to:
- buffalogs/impossible_travel/modules/alert_filter.py
- buffalogs/impossible_travel/tests/detection/test_alert_filter.py

Changes are purely cosmetic (line breaks, import formatting) to meet
Black 25.1.0 code style requirements used in CI.
@kami922 kami922 force-pushed the fix/redos-vulnerability-regex-validation branch from ce3f01b to 3f72991 Compare January 3, 2026 18:45
Fix CI linting failures by sorting imports according to project isort
configuration. Changes apply isort 6.0.1 formatting to:
- buffalogs/impossible_travel/modules/alert_filter.py
- buffalogs/impossible_travel/tests/detection/test_alert_filter.py

Imports are now correctly ordered and formatted to fit within the
160-character line length limit.
@Lorygold
Copy link
Collaborator

Lorygold commented Jan 4, 2026

Ok, great, let's discuss a few doubts:

  1. Shouldn't these checks be performed when the config entry is updated? That way, in the alert_filter process only the regex check would need to be done, instead of running all these checks again.

  2. What about the values already present in the config entry?

@kami922
Copy link
Contributor Author

kami922 commented Jan 5, 2026

@Lorygold Hello
1- Yes, I think it would be a better approach. Moving the _is_safe_regex() check to Config.clean() ensures we validate once at the entry point,ensuring we validate once at the entry point. I shall implement it in next commit
2- To handle legacy data, I will create a management command to scan and log any existing dangerous patterns for admin review. The alert filter will retain a simple try-except as a safety fallback, but the primary validation burden will shift to the config level

let me know if it sounds reasonable to you so i will implement it

@Lorygold
Copy link
Collaborator

Lorygold commented Jan 6, 2026

@kami922

1- Great!
2- I think that, instead of a Django command that would need to be run manually, it would be more appropriate to have something automatic that checks all existing values and reports the presence of vulnerable ones (as described in your analysis), without modifying them.

To achieve this, I was thinking we could update migration 0022 - which should be the latest one - to also call the check function at the end, in the same way we already do in that migration for the populate_device_fingerprint function.

What do you think? Do you see any better alternatives?

@Lorygold Lorygold marked this pull request as draft January 7, 2026 11:35
@kami922
Copy link
Contributor Author

kami922 commented Jan 7, 2026

@Lorygold Hello work is under progress but halted due to my ongoing exams i will try to push fix over the weekend. May I ask why this Pr was marked as draft.

@Lorygold
Copy link
Collaborator

Lorygold commented Jan 8, 2026

No problem! It’s marked as a draft simply because it’s not finished yet, so it serves as a reminder that it shouldn’t be merged for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security] Backend: ReDoS vulnerability in regex pattern validation

2 participants