fix: add ReDoS protection to regex pattern validation

[kami922]

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]

black linter failed in the CI

[Lorygold]

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]

@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]

@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?

[kami922]

@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]

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

[kami922]

1. Validation at Config Save Time

Added validate_regex_patterns() validator to validators.py that uses _is_safe_regex() to check patterns. This validator is now applied to the ignored_users, enabled_users, and vip_users fields in the Config model.

Result: When admins try to save configs with unsafe patterns via the Django admin panel, they'll get a clear ValidationError explaining why the pattern was rejected. No repeated validation during alert processing.

2. Check Existing Configs in Migration 0022

Added check_regex_patterns() function to migration 0022 following the same pattern as populate_device_fingerprint(). The function:

Iterates through all existing Config entries
Checks for unsafe patterns in the three user list fields
Logs warnings with details about which patterns are unsafe
Non-blocking: Doesn't prevent migration from completing
Added to operations:
migrations.RunPython(check_regex_patterns, migrations.RunPython.noop),

[Lorygold]

this file should be moved in the docs/guides/development folder

[Lorygold]

remove all the AI emoticons everywhere, please

[Lorygold]

all the imports should be at the beginning of the file, it's more readable

[Lorygold]

using set() could avoid duplicates:

unsafe = set()  

    for p in patterns_list:
        if not _is_safe_regex(p):
            unsafe.add(p)

[Lorygold]

isn't this check a duplication?

[kami922]

@Lorygold Hello I removed REDOS_FIX_DOCUMENTATION.md from commit these are my personnel notes and accidentally went into commit

[Lorygold]

why all those files have been changed? some problems with the linters version locally and in the CI?

[kami922]

@Lorygold TBH I could not really figure it out I did apply pre commit hook and it said the linting tests passed locally, but failed on github actions.
I applied Black formatting multiple times before committing and pushing and it passed locally but for some reason it failed online adjusted versions still same problem. CI enforces Black (line-length 160) and isort, so once we aligned with the repo’s configs the formatter touched many files. The functional change is small—moving _is_safe_regex into validators to break a circular import—everything else is formatting to satisfy the existing linting rules.

[Lorygold]

maybe because you change this version?

[Lorygold]

you should apply changes only to the files needed for your features, not all those 81 files

[kami922]

@Lorygold Hello I have made a clean commit and hope it works
now regarding the issues associated with formatters I have observed some inconsistencies among versions

Current Versions

Pre-commit config (.github/.pre-commit-config.yaml):

• Black: 24.8.0
• isort: 5.13.2
• flake8: 7.1.1

CI workflow (.github/workflows/_python.yml):

• Black: 25.1.0
• isort: 6.0.1
• flake8: 7.1.2

I think it should be consistent also I would recommend shifting to ruff I am currently working on this issue in intel owl. however it can be a time consuming issue requiring discipline and also would require changing in all files.

[Lorygold]

If you aligned your branch with the develop one, it should be already fixed, but feel free to let me know if the error is still there and I missed something.
The problem persists only for the buffacli_CI that I'll update it now

[kami922]

@Lorygold Hello ok thats great to know let me know whats left for pr to be closed and merged

[Lorygold]

please define all the imports at the beginning of the file

[Lorygold]

also @Noble-47 will give a check soon

[kami922]

@Lorygold Hello any update on this one?

[Noble-47]

Hi @kami922

I'll give you an update soon.