diff --git a/CHANGELOG.d/document-diff-review-data-uri-redos.md b/CHANGELOG.d/document-diff-review-data-uri-redos.md new file mode 100644 index 000000000..936983dd1 --- /dev/null +++ b/CHANGELOG.d/document-diff-review-data-uri-redos.md @@ -0,0 +1 @@ +Fixed CodeQL `py/polynomial-redos` in `document_diff_review`: the inline data-URI leak check used an unbounded `data:[^,\s]*,`, which `search()` rescans to the end of the text from every `data:` occurrence. A fixed-token state scan now recognizes the same data-URI language in linear time without a header-length cutoff, so long valid data URIs still fail closed as `inline_binary_content`. diff --git a/contextual_orchestrator/document_diff_review.py b/contextual_orchestrator/document_diff_review.py index eb0e6d970..fc305e741 100644 --- a/contextual_orchestrator/document_diff_review.py +++ b/contextual_orchestrator/document_diff_review.py @@ -89,7 +89,7 @@ _REPO = re.compile(r"[A-Za-z0-9_.-]{1,100}/[A-Za-z0-9_.-]{1,100}") _BLOB = re.compile(r"[0-9a-f]{40}(?:[0-9a-f]{24})?") _OBJECT_HASH = re.compile(r"sha256:[0-9a-f]{64}") -_DATA_URI = re.compile(r"data:[^,\s]*,", re.IGNORECASE) +_DATA_URI_TOKEN = re.compile(r"data:|[,\s]", re.IGNORECASE) _BASE64_RUN = re.compile(r"[A-Za-z0-9+/_-]{200,}={0,2}") _RESIDENT_REGISTRATION_NUMBER = re.compile(r"(? DocumentDiffReviewErr return DocumentDiffReviewError(code, message, status) +def _contains_data_uri(value: str) -> bool: + """Recognize a data URI in one token scan without a header-length cutoff.""" + header_open = False + for match in _DATA_URI_TOKEN.finditer(value): + token = match.group() + if token.casefold() == "data:": + header_open = True + elif token == ",": + if header_open: + return True + else: + header_open = False + return False + + def _exact_object(value: Any, allowed: frozenset[str], field: str, *, required: frozenset[str]) -> dict[str, Any]: """Require a JSON object whose keys are known and whose required keys exist.""" if type(value) is not dict: @@ -126,7 +141,7 @@ def _exact_object(value: Any, allowed: frozenset[str], field: str, *, required: def _scan_for_leaks(value: str, field: str) -> None: """Reject inline binary/media, credentials, and resident identifiers.""" - if any(signature in value for signature in _BINARY_SIGNATURES) or _DATA_URI.search(value) or _BASE64_RUN.search(value): + if any(signature in value for signature in _BINARY_SIGNATURES) or _contains_data_uri(value) or _BASE64_RUN.search(value): raise _reject("inline_binary_content", f"{field} must not carry inline binary or media data", 422) if any(pattern.search(value) for pattern in SECRET_PATTERNS): raise _reject("secret_detected", f"{field} matches a credential pattern", 422) diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 9d5215cdb..aabcca14c 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -7492,6 +7492,25 @@ test-owned listening socket. The fixture now calls `server_close()` in `finally` after the serving thread joins. This changes no production server lifecycle policy and keeps ResourceWarning visible as a failure signal. +## 2026-09-30 Data-URI leak scan without a heuristic cutoff + +PR [#1340](https://github.com/ContextualWisdomLab/contextual-orchestrator/pull/1340) +at reviewed head `240ce0845e126d11843e70ecd09ca913b7b7800d` removed CodeQL +`py/polynomial-redos` by limiting the data-URI header to 256 characters. That +made runtime linear but introduced an undocumented security decision: a valid +data URI with a longer header was treated as ordinary text and could cross the +document-review boundary. The existing test explicitly approved that false +negative. + +The proposed owner repair replaces the bounded expression with a single scan +over fixed `data:`, comma, and whitespace tokens. It recognizes the original +`data:[^,\s]*,` language without rescanning suffixes and without a header-length +threshold. A header beyond the former cutoff was observed RED before the +repair; the focused long-header, repeated-prefix, whitespace, and varied-input +contracts are GREEN afterward. Protected exact-head CodeQL/Security Checks, +independent approval, and ordinary merge remain required, so this record is +**Proposed** rather than released evidence. + ## 2026-09-30 PR #1348 SearXNG secret and advisory transport repair ### 2026-10-09 exact-revision vulnerability evidence contract — Proposed diff --git a/tests/test_document_diff_review.py b/tests/test_document_diff_review.py index b3df584a4..ca37fea91 100644 --- a/tests/test_document_diff_review.py +++ b/tests/test_document_diff_review.py @@ -14,6 +14,8 @@ import hashlib import io import json +import random +import re import sys import threading import urllib.error @@ -28,7 +30,9 @@ from contextual_orchestrator import ModelAgent, TaskOrchestrator # noqa: E402 from contextual_orchestrator.document_diff_review import ( # noqa: E402 + _contains_data_uri, DocumentDiffReviewError, + _scan_for_leaks, validate_document_diff_envelope, validate_document_diff_findings, ) @@ -261,6 +265,7 @@ def _mutated(**changes) -> dict: [ (_mutated(**{"objects.0.head_text": "data:image/png;base64,iVBORw0KGgo="}), 422, "inline_binary_content"), (_mutated(**{"objects.0.head_text": "data:image/png;name=" + "x" * 101 + ";base64,AAAA"}), 422, "inline_binary_content"), + (_mutated(**{"objects.0.head_text": "DATA:,x"}), 422, "inline_binary_content"), (_mutated(**{"objects.0.head_text": base64.b64encode(HEAD_DOCX).decode()[:4000]}), 422, "inline_binary_content"), ( _mutated(**{"objects.0.head_text": base64.urlsafe_b64encode(b"\x89PNG\r\n\x1a\n" + bytes(range(256)) * 8).decode()}), @@ -302,6 +307,7 @@ def _mutated(**changes) -> dict: ids=[ "data_uri", "long_data_uri_header", + "uppercase_empty_data_uri", "base64_docx", "urlsafe_base64_image", "secret", @@ -473,3 +479,60 @@ def test_total_extracted_text_budget_rejects_individually_bounded_objects() -> N validate_document_diff_envelope(envelope) assert (error.value.status, error.value.code) == (413, "request_too_large") assert "envelope text" in str(error.value) + + +def test_data_uri_scan_handles_repeated_scheme_without_comma() -> None: + """A repeated scheme without a comma is not inline media.""" + pathological = "data:" * 20_000 + assert not _contains_data_uri(pathological) + _scan_for_leaks(pathological, "objects[0].head_text") + + +@pytest.mark.parametrize( + "value", + [ + "data:image/png;base64,iVBORw0KGgo=", + "DATA:,x", + "Data:text/plain;charset=utf-8,hello", + "see data:,x inline", + "data:" + "a;" * 128 + ",AAAA", + "data:" + "a;" * 150 + ",AAAA", + ], + ids=[ + "png_base64", + "uppercase_empty", + "mixed_case_charset", + "embedded", + "header_at_256_bound", + "header_over_256_bound", + ], +) +def test_data_uri_is_rejected_as_inline_binary(value) -> None: + with pytest.raises(DocumentDiffReviewError) as error: + _scan_for_leaks(value, "objects[0].head_text") + assert (error.value.status, error.value.code) == (422, "inline_binary_content") + + +@pytest.mark.parametrize( + "value", + [ + "data:" + "a;" * 150, + "data: image/png, not a uri", + "the data, as reported", + ], + ids=["long_header_without_comma", "whitespace_in_header", "no_scheme"], +) +def test_non_data_uri_text_is_not_flagged(value) -> None: + """Whitespace or an absent comma prevents a data-URI match.""" + assert not _contains_data_uri(value) + _scan_for_leaks(value, "objects[0].head_text") + + +def test_data_uri_scanner_matches_the_contract_on_varied_short_text() -> None: + """The linear scanner preserves the original data-URI language.""" + unbounded = re.compile(r"data:[^,\s]*,", re.IGNORECASE) + rng = random.Random(1262) + alphabet = ["data:", "DATA:", "Data:", ",", ";", "/", " ", "\n", "a", "b64", "="] + for _ in range(3000): + value = "".join(rng.choice(alphabet) for _ in range(rng.randint(0, 40))) + assert _contains_data_uri(value) == bool(unbounded.search(value)), value