Fix ReDoS vulnerability (py/redos) in path validation - Security Alert #6

packages/extended-data/src/extended_data/primitives/types.py

@@ -54,7 +54,10 @@
     r"^\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}(:\d{2})?(?:\.\d+)?(?:Z|[+-]\d{2}:\d{2})?$"
 )  # Matches extended datetime formats like YYYY-MM-DDTHH:MM[:SS][.fff][Z|±hh:mm]
 TIME_PATTERN: re.Pattern[str] = re.compile(r"^\d{2}:\d{2}(:\d{2}(\.\d{1,6})?)?$")  # Matches HH:MM[:SS] and microseconds
-PATH_PATTERN: re.Pattern[str] = re.compile(r'^(?:[a-zA-Z]:)?[\\/](?:[^<>:"|?*\n]+[\\/])*[^<>:"|?*\n]*$')
+# Path validation pattern - simplified to avoid ReDoS vulnerability (CWE-1333)
+# Matches absolute paths starting with / or \ (or drive letter on Windows)
+# Forbidden characters are validated separately to prevent catastrophic backtracking
+PATH_PATTERN: re.Pattern[str] = re.compile(r'^(?:[a-zA-Z]:)?[\\/]')
 INTEGER_PATTERN: re.Pattern[str] = re.compile(r"^-?\d+$")
 NUMBER_PATTERN: re.Pattern[str] = re.compile(r"^-?\d+(\.\d+)?$")
 TRUTHY_PATTERN: re.Pattern[str] = re.compile(r"^(y|yes|t|true|on|1)$", re.IGNORECASE)
@@ -202,6 +205,23 @@ def string_to_int(val: str, raise_on_error: bool = False) -> int | None:
 def string_to_path(val: str | bytes | os.PathLike[str] | None, raise_on_error: bool = False) -> Path | None:
     """Converts a string or byte representation of a path to a pathlib.Path object.
 
+def _is_valid_path_string(val: str) -> bool:
+    """Check if a string is a valid path without using complex regex.
+
+    Args:
+        val: String to validate as a path.
+
+    Returns:
+        True if the string represents a valid absolute path, False otherwise.
+    """
+    # Must start with / or \ (or drive letter on Windows)
+    if not PATH_PATTERN.match(val):
+        return False
+    # Must not contain forbidden characters or newlines
+    forbidden_chars = '<>:"|?*'
+    return not any(char in val for char in forbidden_chars) and '\n' not in val
+
+
     Args:
         val (str | bytes | pathlib.Path | None): The value to convert.
         raise_on_error (bool): Whether to raise an error on invalid value. Defaults to False.
@@ -224,7 +244,7 @@ def string_to_path(val: str | bytes | os.PathLike[str] | None, raise_on_error: b
                 return None
         # Ensure val is converted to string before matching
         val = str(val)
-        if not PATH_PATTERN.match(val):
+        if not _is_valid_path_string(val):
             raise ConversionError(Path, val)
         return Path(val)
     except (ValueError, TypeError) as exc:
@@ -436,8 +456,9 @@ def reconstruct_special_type(converted_obj: str, fail_silently: bool = False) ->
         if TIME_PATTERN.match(converted_obj):
             return string_to_time(converted_obj)
         if PATH_PATTERN.match(converted_obj):
-            return pathlib.Path(converted_obj)
-        if TRUTHY_PATTERN.match(converted_obj) or FALSY_PATTERN.match(converted_obj):
+        # Use the safe path validation helper
+        if _is_valid_path_string(converted_obj):
+            return string_to_path(converted_obj)
             return string_to_bool(converted_obj)
         if NUMBER_PATTERN.match(converted_obj):
             if INTEGER_PATTERN.match(converted_obj):

packages/extended-data/tests/core/test_type_utils.py

@@ -8,6 +8,7 @@
 from __future__ import annotations
 
 import datetime
+import time
 
 from pathlib import Path
 from typing import Any
@@ -803,3 +804,65 @@ def __str__(self) -> str:
 
         result = make_hashable(CustomClass())
         assert result == "custom_str"
+
+
+class TestReDoSRegression:
+    """Tests for ReDoS vulnerability fixes (GitHub Security Alert #6)."""
+
+    def test_path_validation_completes_quickly_with_many_chars_followed_by_invalid(self) -> None:
+        """Ensure path validation doesn't cause catastrophic backtracking.
+
+        This test addresses CWE-1333 (Inefficient Regular Expression Complexity).
+        Before the fix, the PATH_PATTERN regex had nested quantifiers that could
+        cause exponential time complexity with certain malicious inputs.
+
+        The vulnerable pattern was: r'^(?:[a-zA-Z]:)?[\\/](?:[^<>:"|?*\n]+[\\/])*[^<>:"|?*\n]*$'
+        With nested quantifiers on character sets that could overlap.
+        """
+        # This string would cause ReDoS: many valid path characters followed by invalid
+        malicious_input = "/valid" + "a" * 50 + "!"
+
+        start_time = time.time()
+        result = string_to_path(malicious_input)
+        elapsed_time = time.time() - start_time
+
+        # Should complete in well under 1 second (ReDoS would take exponentially longer)
+        assert elapsed_time < 1.0, f"Path validation took {elapsed_time}s, possible ReDoS"
+        assert result is None, "Invalid path should return None"
+
+    def test_path_validation_rejects_forbidden_characters(self) -> None:
+        """Verify that paths with forbidden characters are rejected."""
+        forbidden_paths = [
+            "/path/with:colon",
+            "/path/with<angle",
+            '/path/with"quote',
+            "/path/with|pipe",
+            "/path/with?question",
+            "/path/with*asterisk",
+            "/path/with\nnewline",
+        ]
+
+        for bad_path in forbidden_paths:
+            assert string_to_path(bad_path) is None, f"Should reject path with forbidden char: {bad_path}"
+
+    def test_path_validation_accepts_legitimate_paths(self) -> None:
+        """Verify that legitimate paths still work after the fix."""
+        valid_paths = [
+            "/valid/unix/path",
+            "/path-with-dashes",
+            "/path_with_underscores",
+            "/path.with.dots",
+            "/path/with spaces",
+            "C:\\windows\\path",
+            "D:/mixed/slashes",
+        ]
+
+        for good_path in valid_paths:
+            result = string_to_path(good_path)
+            assert result is not None, f"Should accept valid path: {good_path}"
+            assert isinstance(result, Path)
+
+    def test_reconstruct_special_type_uses_safe_path_validation(self) -> None:
+        """Ensure reconstruct_special_type also uses the safe path validation."""
+        result = reconstruct_special_type("/valid/path")
+        assert isinstance(result, Path)