diff --git a/backend/app/core/logging_filters.py b/backend/app/core/logging_filters.py index 872f03aad..d64f3a642 100644 --- a/backend/app/core/logging_filters.py +++ b/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[a-zA-Z][a-zA-Z0-9+.\-]*://)(?P[^/:@\s]+):(?P[^/\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[a-zA-Z][a-zA-Z0-9+.\-]{0,63}://)(?P[^/:@\s]+):(?P[^/\s]+)@" +) def redact_url_credentials(text: str | None) -> str | None: diff --git a/backend/tests/unit/test_log_credential_redaction.py b/backend/tests/unit/test_log_credential_redaction.py index 25bb29fc2..2cfea5070 100644 --- a/backend/tests/unit/test_log_credential_redaction.py +++ b/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