فهرست منبع

Bound the scheme repetition in the log credential-redaction pattern. As an
unbounded repetition the match was quadratic in the subject length: on a run
of scheme-legal characters the engine restarted at every offset and consumed
to the end before failing to find "://". ffmpeg echoes the configured camera
URL into its stderr and the whole blob reaches the pattern before any
truncation, so the subject length is attacker-influenced.

maziggy 1 ماه پیش
والد
کامیت
bbbb9d35c7
2فایلهای تغییر یافته به همراه52 افزوده شده و 1 حذف شده
  1. 14 1
      backend/app/core/logging_filters.py
  2. 38 0
      backend/tests/unit/test_log_credential_redaction.py

+ 14 - 1
backend/app/core/logging_filters.py

@@ -27,7 +27,20 @@ import re
 # external camera URL) from leaving its tail in the log. Named groups let
 # callers choose how much to mask: the log pipeline keeps the username, the
 # support-bundle sanitizer drops it (see ``log_reader.sanitize_log_content``).
-URL_CREDENTIALS_PATTERN = re.compile(r"(?P<scheme>[a-zA-Z][a-zA-Z0-9+.\-]*://)(?P<user>[^/:@\s]+):(?P<secret>[^/\s]+)@")
+#
+# The scheme's repetition is bounded deliberately. As an unbounded ``*`` the
+# match was quadratic in the length of the subject (CodeQL py/polynomial-redos):
+# on a long run of scheme-legal characters the engine restarts at every offset
+# and consumes to the end each time before failing to find ``://``. Measured at
+# 557ms for a 32KB line, quadrupling per doubling. ffmpeg echoes the operator's
+# camera URL back in its stderr, and that whole string reaches this pattern
+# before any truncation, so the subject length is attacker-influenced. A cap
+# makes the work per offset constant. 63 is far above any real scheme (the
+# longest registered one is under 20 characters), and a longer pseudo-scheme
+# still gets its secret masked — the match simply starts from a later offset.
+URL_CREDENTIALS_PATTERN = re.compile(
+    r"(?P<scheme>[a-zA-Z][a-zA-Z0-9+.\-]{0,63}://)(?P<user>[^/:@\s]+):(?P<secret>[^/\s]+)@"
+)
 
 
 def redact_url_credentials(text: str | None) -> str | None:

+ 38 - 0
backend/tests/unit/test_log_credential_redaction.py

@@ -10,6 +10,7 @@ into the log.
 """
 
 import asyncio
+import time
 
 from backend.app.api.routes.camera import _read_ffmpeg_stderr, _summarize_ffmpeg_stderr
 from backend.app.core.logging_filters import redact_url_credentials
@@ -71,6 +72,43 @@ class TestRedactUrlCredentials:
         assert redact_url_credentials("") == ""
         assert redact_url_credentials(None) is None
 
+    def test_a_long_scheme_like_run_does_not_blow_up(self):
+        """The scheme repetition is capped so the match stays linear.
+
+        Unbounded, the engine restarted at every offset of a run of
+        scheme-legal characters and consumed to the end each time before
+        failing to find ``://`` — quadratic in the length of the line, and
+        ffmpeg echoes the operator's camera URL into the subject. An absolute
+        timing bound would be flaky, so this pins the growth rate instead:
+        doubling the input must not quadruple the work. Measured against the
+        unbounded pattern, these two inputs took 550ms and 2187ms (ratio 3.97,
+        so the assertion fails); bounded, 2.8ms and 5.4ms (ratio 1.98).
+        """
+        small = "A" * 32_000 + "://@"
+        large = "A" * 64_000 + "://@"
+
+        start = time.perf_counter()
+        assert redact_url_credentials(small) == small
+        small_elapsed = time.perf_counter() - start
+
+        start = time.perf_counter()
+        assert redact_url_credentials(large) == large
+        large_elapsed = time.perf_counter() - start
+
+        # Linear would be ~2x. Allow generous slack for a loaded CI box while
+        # still failing the ~4x of a quadratic match.
+        assert large_elapsed < max(small_elapsed * 3, 0.5)
+
+    def test_a_scheme_longer_than_the_cap_still_gets_its_secret_masked(self):
+        """The cap bounds backtracking; it must not create a redaction hole.
+
+        A pseudo-scheme longer than the cap simply matches from a later
+        offset, so the password is still replaced.
+        """
+        result = redact_url_credentials("Z" * 100 + "://user:hunter2@host/path")
+        assert "hunter2" not in result
+        assert result.endswith("://user:[REDACTED]@host/path")
+
 
 class TestFfmpegStderrFunnel:
     """`_summarize_ffmpeg_stderr` is the one funnel every stderr log in the