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.
This commit is contained in:
maziggy 2026-08-02 11:17:24 +02:00
parent 3b5d219860
commit 3da4eee16e
2 changed files with 52 additions and 1 deletions

View file

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

View file

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