diff --git a/src/youtube_extension/backend/config/logging_config.py b/src/youtube_extension/backend/config/logging_config.py index 98f28489e..654252333 100644 --- a/src/youtube_extension/backend/config/logging_config.py +++ b/src/youtube_extension/backend/config/logging_config.py @@ -15,6 +15,23 @@ from pathlib import Path +# CWE-117 (log injection / log forging): line-breaking characters that let a +# user-controlled value spawn a second, forged log line. Neutralized in the +# FINAL rendered record — see StructuredFormatter.format() — so exc_info +# tracebacks and structured ``extra`` fields are covered even when a value was +# not sanitized at the call site. Escaping (rather than dropping) keeps the +# content greppable while guaranteeing one log call renders as one physical line. +_LOG_FORGERY_ESCAPES = { + 0x0A: "\\n", # line feed + 0x0D: "\\r", # carriage return + 0x0B: "\\v", # vertical tab + 0x0C: "\\f", # form feed + 0x85: "\\x85", # NEL (Unicode next line) + 0x2028: "\\u2028", # line separator + 0x2029: "\\u2029", # paragraph separator +} + + class StructuredFormatter(logging.Formatter): """ Custom formatter for structured logging with enhanced metadata. @@ -39,7 +56,12 @@ def format(self, record: logging.LogRecord) -> str: # Format the base message formatted_message = super().format(record) - return formatted_message + # CWE-117: neutralize CR/LF (and other line separators) in the fully + # rendered record. This covers the message, any exc_info traceback text + # (including ``str(exc)``) and structured ``extra`` fields the format + # string references, so a user-controlled value carrying "\r\n" cannot + # forge an additional log line even when it was not sanitized inline. + return formatted_message.translate(_LOG_FORGERY_ESCAPES) def formatException(self, ei) -> str: """Format exception with enhanced stack trace""" diff --git a/tests/unit/test_logging_config_crlf.py b/tests/unit/test_logging_config_crlf.py new file mode 100644 index 000000000..284d6412c --- /dev/null +++ b/tests/unit/test_logging_config_crlf.py @@ -0,0 +1,55 @@ +"""CWE-117 regression: StructuredFormatter must neutralize log forging. + +These tests assert against the *rendered* handler output (not a sanitizer's +return value), covering the exc_info / logger.exception sinks that append raw +traceback and exception text after the formatted message. +""" + +import io +import logging + +import pytest + +from youtube_extension.backend.config.logging_config import StructuredFormatter + + +def _render(logger_call) -> str: + buf = io.StringIO() + handler = logging.StreamHandler(buf) + handler.setFormatter(StructuredFormatter("%(levelname)s - %(message)s")) + logger = logging.getLogger("crlf-regression") + logger.handlers[:] = [handler] + logger.propagate = False + logger.setLevel(logging.DEBUG) + logger_call(logger) + handler.flush() + return buf.getvalue() + + +def test_crlf_in_message_cannot_forge_a_line(): + out = _render(lambda lg: lg.info("video %s captured", "abc\r\nCRITICAL - FORGED")) + # Exactly one physical line (plus the trailing newline the handler adds). + assert out.count("\n") == 1 + assert "\r" not in out + assert "FORGED" in out # content preserved, just neutralized + assert "CRITICAL - FORGED" not in out.splitlines() # not a standalone line + + +def test_exc_info_traceback_cannot_forge_log_lines(): + def call(lg): + try: + raise ValueError("boom\r\nCRITICAL - FORGED ADMIN LINE") + except ValueError: + lg.error("Error in chat endpoint", exc_info=True) + + out = _render(call) + assert "\r" not in out + # The forged text must never appear as its own rendered line. + assert "CRITICAL - FORGED ADMIN LINE" not in out.splitlines() + + +@pytest.mark.parametrize("sep", ["
", "
", "\x0b", "\x0c", "\x85"]) +def test_unicode_line_separators_are_neutralized(sep): + out = _render(lambda lg: lg.info("session %s", "s" + sep + "INJECTED")) + assert sep not in out + assert "INJECTED" in out