Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# Generated by Django 5.2.4 on 2025-12-01 13:49

from django.db import migrations, models
from ua_parser import user_agent_parser
import logging

from django.db import migrations, models
from impossible_travel.models import Login
import logging
from ua_parser import user_agent_parser

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -81,6 +81,31 @@ def populate_device_fingerprint(apps, schema_editor):
Login.objects.bulk_update(logins_to_update, ["device_fingerprint"])


def check_regex_patterns(apps, schema_editor):
"""Check existing Config entries for unsafe regex patterns and log warnings.

This is a non-blocking check that identifies existing dangerous patterns
in ignored_users, enabled_users, and vip_users fields. Admins should
review and update these patterns in the Django admin panel.
"""
Config = apps.get_model("impossible_travel", "Config")
from impossible_travel.validators import _is_safe_regex
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please define all the imports at the beginning of the file


for config in Config.objects.all():
for field_name in ["ignored_users", "enabled_users", "vip_users"]:
patterns = getattr(config, field_name, []) or []
if not patterns:
continue

unsafe = [p for p in patterns if not _is_safe_regex(p)]
if unsafe:
logger.warning(
f"[ReDoS Security Check] Config {config.id}: Found {len(unsafe)} unsafe regex pattern(s) "
f"in '{field_name}' field: {unsafe}. These patterns should be reviewed and updated in the "
f"Django admin panel to prevent potential Denial of Service attacks. See migration 0022 for details."
)


class Migration(migrations.Migration):

dependencies = [
Expand All @@ -102,4 +127,5 @@ class Migration(migrations.Migration):
constraint=models.UniqueConstraint(fields=("task_name", "execution_mode"), name="unique_task_execution"),
),
migrations.RunPython(populate_device_fingerprint, migrations.RunPython.noop),
migrations.RunPython(check_regex_patterns, migrations.RunPython.noop),
]
9 changes: 6 additions & 3 deletions buffalogs/impossible_travel/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
validate_countries_names,
validate_country_couples_list,
validate_ips_or_network,
validate_regex_patterns,
validate_string_or_regex,
validate_tags,
)
Expand Down Expand Up @@ -274,22 +275,23 @@ class Config(models.Model):
blank=True,
null=True,
default=get_default_ignored_users,
validators=[validate_string_or_regex],
validators=[validate_string_or_regex, validate_regex_patterns],
help_text="List of users (strings or regex patterns) to be ignored from the detection",
)
enabled_users = ArrayField(
models.CharField(max_length=50),
blank=True,
null=True,
default=get_default_enabled_users,
validators=[validate_string_or_regex],
validators=[validate_string_or_regex, validate_regex_patterns],
help_text="List of selected users (strings or regex patterns) on which the detection will perform",
)
vip_users = ArrayField(
models.CharField(max_length=50),
blank=True,
null=True,
default=get_default_vip_users,
validators=[validate_regex_patterns],
help_text="List of users considered more sensitive",
)
alert_is_vip_only = models.BooleanField(
Expand Down Expand Up @@ -385,7 +387,8 @@ class Config(models.Model):
help_text="Days after which a login from a country is considered atypical",
)
user_learning_period = models.PositiveIntegerField(
default=settings.CERTEGO_BUFFALOGS_USER_LEARNING_PERIOD, help_text="Days considered to learn the user login behaviors - no alerts generation"
default=settings.CERTEGO_BUFFALOGS_USER_LEARNING_PERIOD,
help_text="Days considered to learn the user login behaviors - no alerts generation",
)
user_max_days = models.PositiveIntegerField(
default=settings.CERTEGO_BUFFALOGS_USER_MAX_DAYS,
Expand Down
132 changes: 128 additions & 4 deletions buffalogs/impossible_travel/tests/validators/test_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
validate_datetime_str,
validate_ips_or_network,
validate_login_query,
validate_regex_patterns,
validate_risk_score,
validate_string_or_regex,
validate_tags,
Expand Down Expand Up @@ -132,13 +133,19 @@ def test_validate_country_couples_list_single_list(self):
"""Test that a single list (not a list of lists) raises an exception"""
with self.assertRaises(ValidationError) as context:
validate_country_couples_list(["Italy", "Germany"])
self.assertIn("Each single value must be a list of 2 elements (list of lists).", str(context.exception))
self.assertIn(
"Each single value must be a list of 2 elements (list of lists).",
str(context.exception),
)

def test_validate_country_couples_list_too_elements(self):
"""Test that a list of more than 2 elements raises an exception"""
with self.assertRaises(ValidationError) as context:
validate_country_couples_list([["Italy", "Germany", "France"]])
self.assertIn("Each single value must be a list of 2 elements (list of lists).", str(context.exception))
self.assertIn(
"Each single value must be a list of 2 elements (list of lists).",
str(context.exception),
)

def test_validate_country_couples_list_wrong_country_name(self):
"""Test that a list containing a wrong country name raises an exception"""
Expand Down Expand Up @@ -226,11 +233,17 @@ def test_validate_risk_score_valid(self):
self.assertEqual(validate_risk_score("high"), "High")

# Test empty string
with self.assertRaisesRegex(ValidationError, "Risk score must be an integer 0-7 or one of: High, Medium, Low, No Risk"):
with self.assertRaisesRegex(
ValidationError,
"Risk score must be an integer 0-7 or one of: High, Medium, Low, No Risk",
):
validate_risk_score("")

# Test empty string
with self.assertRaisesRegex(ValidationError, "Risk score must be an integer 0-7 or one of: High, Medium, Low, No Risk"):
with self.assertRaisesRegex(
ValidationError,
"Risk score must be an integer 0-7 or one of: High, Medium, Low, No Risk",
):
validate_risk_score("Invalid Risk Score")

# Test out of range risk score
Expand Down Expand Up @@ -372,3 +385,114 @@ def test_validate_tags(self):

with self.assertRaises(ValidationError):
validate_tags("security_threat") # Tags must be passed as Lists

def test_is_safe_regex_valid_patterns(self):
"""Test that safe regex patterns are accepted."""
from impossible_travel.validators import _is_safe_regex

safe_patterns = [
r"^admin.*",
r"test\d+",
r"user@.*\.com",
r"[a-z]+@domain\.com",
"admin", # Plain string
"test-user", # Plain string with hyphen
]

for pattern in safe_patterns:
with self.subTest(pattern=pattern):
self.assertTrue(_is_safe_regex(pattern), f"Pattern should be safe: {pattern}")

def test_is_safe_regex_rejects_nested_quantifiers(self):
"""Test that patterns with nested quantifiers are rejected (ReDoS vulnerability)."""
from impossible_travel.validators import _is_safe_regex

dangerous_patterns = [
r"(a+)+", # Nested plus quantifiers
r"(a*)*", # Nested star quantifiers
r"(a+)*", # Mixed nested quantifiers
r"(a|ab)*", # Alternation with quantifier
r"(a|a)+", # Redundant alternation with quantifier
]

for pattern in dangerous_patterns:
with self.subTest(pattern=pattern):
self.assertFalse(_is_safe_regex(pattern), f"Dangerous pattern should be rejected: {pattern}")

def test_is_safe_regex_rejects_too_long(self):
"""Test that patterns exceeding MAX_REGEX_LENGTH are rejected."""
from impossible_travel.validators import _is_safe_regex

# Pattern longer than 100 characters
long_pattern = "a" * 101
self.assertFalse(_is_safe_regex(long_pattern), "Pattern exceeding 100 chars should be rejected")

# Pattern exactly 100 characters (should pass length check)
exactly_100 = "a" * 100
self.assertTrue(_is_safe_regex(exactly_100), "Pattern with exactly 100 chars should pass length check")

def test_is_safe_regex_rejects_too_complex(self):
"""Test that patterns with too many special characters are rejected."""
from impossible_travel.validators import _is_safe_regex

# Pattern with > 50 special regex characters
complex_pattern = "(" * 30 + "a" + ")" * 30 + "*" * 20
self.assertFalse(_is_safe_regex(complex_pattern), "Overly complex pattern should be rejected")

def test_is_safe_regex_rejects_invalid_syntax(self):
"""Test that patterns with invalid regex syntax are rejected."""
from impossible_travel.validators import _is_safe_regex

invalid_patterns = [
r"[unclosed", # Unclosed bracket
r"(unmatched", # Unmatched parenthesis
r"*invalid", # Invalid quantifier placement
r"(?P<incomplete", # Incomplete named group
]

for pattern in invalid_patterns:
with self.subTest(pattern=pattern):
self.assertFalse(_is_safe_regex(pattern), f"Invalid syntax should be rejected: {pattern}")

def test_validate_regex_patterns_accepts_safe_list(self):
"""Test that validate_regex_patterns accepts a list of safe patterns."""
safe_list = [r"^admin.*", r"test\d+", "user@domain.com"]

# Should not raise ValidationError
try:
validate_regex_patterns(safe_list)
except ValidationError:
self.fail("validate_regex_patterns raised ValidationError for safe patterns")

def test_validate_regex_patterns_rejects_unsafe_list(self):
"""Test that validate_regex_patterns rejects lists containing unsafe patterns."""
unsafe_list = [r"^admin.*", r"(a+)+", r"test\d+"] # Middle pattern is unsafe

with self.assertRaises(ValidationError) as context:
validate_regex_patterns(unsafe_list)

self.assertIn("unsafe", str(context.exception).lower())
self.assertIn("(a+)+", str(context.exception))

def test_validate_regex_patterns_accepts_empty_list(self):
"""Test that validate_regex_patterns accepts empty list."""
# Should not raise ValidationError
try:
validate_regex_patterns([])
except ValidationError:
self.fail("validate_regex_patterns raised ValidationError for empty list")

def test_validate_regex_patterns_accepts_none(self):
"""Test that validate_regex_patterns accepts None."""
# Should not raise ValidationError
try:
validate_regex_patterns(None)
except ValidationError:
self.fail("validate_regex_patterns raised ValidationError for None")

def test_validate_regex_patterns_rejects_non_list(self):
"""Test that validate_regex_patterns rejects non-list input."""
with self.assertRaises(ValidationError) as context:
validate_regex_patterns("not a list")

self.assertIn("list", str(context.exception).lower())
83 changes: 83 additions & 0 deletions buffalogs/impossible_travel/validators.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logging
import re
from ipaddress import AddressValueError, IPv4Address, IPv4Network
from typing import Any, Dict, Optional, Union
Expand All @@ -10,6 +11,60 @@
from impossible_travel.views.utils import read_config

ALLOWED_RISK_STRINGS = ["High", "Medium", "Low", "No Risk"]
logger = logging.getLogger(__name__)

# Security constants for ReDoS protection
MAX_REGEX_LENGTH = 100
MAX_REGEX_COMPLEXITY = 50 # Maximum number of special regex characters


def _is_safe_regex(pattern: str) -> bool:
"""
Validates that a regex pattern is safe to compile and execute.
Protects against Regular Expression Denial of Service (ReDoS) attacks.

Args:
pattern: Regular expression pattern string

Returns:
True if pattern is safe, False otherwise
"""
# Check pattern length
if len(pattern) > MAX_REGEX_LENGTH:
logger.warning(f"Regex pattern exceeds maximum length ({MAX_REGEX_LENGTH}): {len(pattern)}")
return False

# Count special regex characters that can cause complexity
dangerous_chars = ["*", "+", "{", "(", "|", "["]
complexity = sum(pattern.count(char) for char in dangerous_chars)

if complexity > MAX_REGEX_COMPLEXITY:
logger.warning(f"Regex pattern too complex ({complexity} special chars, max {MAX_REGEX_COMPLEXITY})")
return False

# Check for known dangerous patterns that can cause catastrophic backtracking
# These patterns check for nested quantifiers which are the primary cause of ReDoS
dangerous_patterns = [
r"\(.+[*+]\)[*+]", # Direct nested quantifiers like (a+)+ or (a*)*
r"\(.+[*+]\).?[*+]", # Nested quantifiers with optional char like (a+)+b
r"\(.+\|.+\)[*+]", # Alternation with quantifier like (a|ab)*
]

for dangerous in dangerous_patterns:
try:
if re.search(dangerous, pattern):
logger.warning(f"Regex pattern contains dangerous construct: {pattern}")
return False
except re.error:
pass

# Try to compile to catch syntax errors
try:
re.compile(pattern)
return True
except re.error as e:
logger.error(f"Invalid regex syntax: {pattern}, error: {e}")
return False


def validate_string_or_regex(value):
Expand All @@ -27,6 +82,34 @@ def validate_string_or_regex(value):
raise ValidationError(f"The single element '{item}' in the '{value}' list field is not a valid regex pattern")


def validate_regex_patterns(patterns_list):
"""Validator for regex patterns - rejects unsafe patterns that could cause ReDoS attacks.

Args:
patterns_list: List of regex pattern strings

Raises:
ValidationError: If any pattern is unsafe
"""
if not patterns_list:
return

if not isinstance(patterns_list, list):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this check a duplication?

raise ValidationError("Must be a list of patterns")

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

if unsafe:
raise ValidationError(
f"The following regex patterns are unsafe and have been rejected: {list(unsafe)}. "
"Patterns must not exceed 100 characters, contain more than 50 special characters, "
"or contain nested quantifiers like (a+)+, (a*)*, or (a|ab)*."
)


def validate_ips_or_network(value):
"""Validator for models' fields list that must have IPs or networks"""
for item in value:
Expand Down
Loading
Loading