From 240ce0845e126d11843e70ecd09ca913b7b7800d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 28 Sep 2026 22:04:43 +0900 Subject: [PATCH 1/2] fix(security): make document_diff_review data-URI regex linear (CodeQL py/polynomial-redos) Bound the data-URI header before the comma to 256 characters so _DATA_URI.search() does at most 256 steps per start position instead of rescanning to the end of the text from every 'data:' occurrence. Adds regression tests: 'data:' * 20000 without a comma finishes fast, real data URIs (including DATA:,x) are still rejected as inline binary, a 300-character header is not flagged, and the bounded pattern agrees with the old one below the bound. --- .../document-diff-review-data-uri-redos.md | 1 + .../document_diff_review.py | 9 ++- tests/test_document_diff_review.py | 59 +++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 CHANGELOG.d/document-diff-review-data-uri-redos.md 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..01019c74b --- /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 `_DATA_URI` used an unbounded `data:[^,\s]*,`, which `search()` rescans to the end of the text from every `data:` occurrence (quadratic on `"data:" * n` without a comma). The header before the comma is now bounded to 256 characters (`data:[^,\s]{0,256},`), so the scan is linear; data URIs with a header of at most 256 characters are still rejected as `inline_binary_content`. diff --git a/contextual_orchestrator/document_diff_review.py b/contextual_orchestrator/document_diff_review.py index eb0e6d970..77f8b199e 100644 --- a/contextual_orchestrator/document_diff_review.py +++ b/contextual_orchestrator/document_diff_review.py @@ -89,7 +89,14 @@ _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) +# The data-URI header (media type and parameters before the comma) is bounded to +# 256 characters. An unbounded ``[^,\s]*`` rescans to the end of the text from +# every ``data:`` occurrence under ``search()``, which is quadratic on +# ``"data:" * n`` without a comma (CodeQL py/polynomial-redos). With the bound each +# start position does at most 256 steps, so the scan is linear in the input length. +# Real media-type headers are far shorter; a header longer than 256 characters is +# not treated as a data URI by this check. +_DATA_URI = re.compile(r"data:[^,\s]{0,256},", re.IGNORECASE) _BASE64_RUN = re.compile(r"[A-Za-z0-9+/_-]{200,}={0,2}") _RESIDENT_REGISTRATION_NUMBER = re.compile(r"(? 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 +308,7 @@ def _mutated(**changes) -> dict: ids=[ "data_uri", "long_data_uri_header", + "uppercase_empty_data_uri", "base64_docx", "urlsafe_base64_image", "secret", @@ -473,3 +480,55 @@ 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_is_linear_on_repeated_scheme_without_comma() -> None: + """CodeQL py/polynomial-redos: ``"data:" * n`` without a comma must not rescan to the end.""" + pathological = "data:" * 20_000 + started = time.perf_counter() + assert _DATA_URI.search(pathological) is None + _scan_for_leaks(pathological, "objects[0].head_text") + assert time.perf_counter() - started < 1.0 + + +@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", + ], + ids=["png_base64", "uppercase_empty", "mixed_case_charset", "embedded", "header_at_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:" + "a;" * 150 + ",AAAA", + "data: image/png, not a uri", + "the data, as reported", + ], + ids=["long_header_without_comma", "header_over_256_bound", "whitespace_in_header", "no_scheme"], +) +def test_non_data_uri_text_is_not_flagged(value) -> None: + """The header is bounded to 256 characters, so a 300-character header is not a data URI here.""" + assert _DATA_URI.search(value) is None + _scan_for_leaks(value, "objects[0].head_text") + + +def test_bounded_data_uri_matches_unbounded_pattern_below_the_header_bound() -> None: + """Below the 256-character bound the linear pattern agrees with the old unbounded one.""" + 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 bool(_DATA_URI.search(value)) == bool(unbounded.search(value)), value From 832291c11da301e919d9dc20fda99f0847142dd8 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 30 Sep 2026 13:10:47 +0900 Subject: [PATCH 2/2] fix(security): scan data URIs without a cutoff Replace the 256-character security cutoff with a fixed-token state scan that recognizes the original data-URI language in linear time. Long valid data URIs remain fail-closed, while repeated data: prefixes without a comma do not trigger quadratic suffix rescans. Add RED-to-GREEN long-header coverage and record the Proposed evidence boundary. --- .../document-diff-review-data-uri-redos.md | 2 +- .../document_diff_review.py | 26 +++++++++----- docs/product-technical-gap-baseline.md | 19 +++++++++++ tests/test_document_diff_review.py | 34 +++++++++++-------- 4 files changed, 56 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.d/document-diff-review-data-uri-redos.md b/CHANGELOG.d/document-diff-review-data-uri-redos.md index 01019c74b..936983dd1 100644 --- a/CHANGELOG.d/document-diff-review-data-uri-redos.md +++ b/CHANGELOG.d/document-diff-review-data-uri-redos.md @@ -1 +1 @@ -Fixed CodeQL `py/polynomial-redos` in `document_diff_review`: the inline data-URI leak check `_DATA_URI` used an unbounded `data:[^,\s]*,`, which `search()` rescans to the end of the text from every `data:` occurrence (quadratic on `"data:" * n` without a comma). The header before the comma is now bounded to 256 characters (`data:[^,\s]{0,256},`), so the scan is linear; data URIs with a header of at most 256 characters are still rejected as `inline_binary_content`. +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 77f8b199e..fc305e741 100644 --- a/contextual_orchestrator/document_diff_review.py +++ b/contextual_orchestrator/document_diff_review.py @@ -89,14 +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}") -# The data-URI header (media type and parameters before the comma) is bounded to -# 256 characters. An unbounded ``[^,\s]*`` rescans to the end of the text from -# every ``data:`` occurrence under ``search()``, which is quadratic on -# ``"data:" * n`` without a comma (CodeQL py/polynomial-redos). With the bound each -# start position does at most 256 steps, so the scan is linear in the input length. -# Real media-type headers are far shorter; a header longer than 256 characters is -# not treated as a data URI by this check. -_DATA_URI = re.compile(r"data:[^,\s]{0,256},", 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: @@ -133,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 a0d26a7d1..55498da2f 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -7491,3 +7491,22 @@ serving loop and closed the injected embedding backend but did not release the 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. diff --git a/tests/test_document_diff_review.py b/tests/test_document_diff_review.py index f44970d4f..ca37fea91 100644 --- a/tests/test_document_diff_review.py +++ b/tests/test_document_diff_review.py @@ -18,7 +18,6 @@ import re import sys import threading -import time import urllib.error import urllib.request import zipfile @@ -31,7 +30,7 @@ from contextual_orchestrator import ModelAgent, TaskOrchestrator # noqa: E402 from contextual_orchestrator.document_diff_review import ( # noqa: E402 - _DATA_URI, + _contains_data_uri, DocumentDiffReviewError, _scan_for_leaks, validate_document_diff_envelope, @@ -482,13 +481,11 @@ def test_total_extracted_text_budget_rejects_individually_bounded_objects() -> N assert "envelope text" in str(error.value) -def test_data_uri_scan_is_linear_on_repeated_scheme_without_comma() -> None: - """CodeQL py/polynomial-redos: ``"data:" * n`` without a comma must not rescan to the end.""" +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 - started = time.perf_counter() - assert _DATA_URI.search(pathological) is None + assert not _contains_data_uri(pathological) _scan_for_leaks(pathological, "objects[0].head_text") - assert time.perf_counter() - started < 1.0 @pytest.mark.parametrize( @@ -499,8 +496,16 @@ def test_data_uri_scan_is_linear_on_repeated_scheme_without_comma() -> None: "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", ], - ids=["png_base64", "uppercase_empty", "mixed_case_charset", "embedded", "header_at_256_bound"], ) def test_data_uri_is_rejected_as_inline_binary(value) -> None: with pytest.raises(DocumentDiffReviewError) as error: @@ -512,23 +517,22 @@ def test_data_uri_is_rejected_as_inline_binary(value) -> None: "value", [ "data:" + "a;" * 150, - "data:" + "a;" * 150 + ",AAAA", "data: image/png, not a uri", "the data, as reported", ], - ids=["long_header_without_comma", "header_over_256_bound", "whitespace_in_header", "no_scheme"], + ids=["long_header_without_comma", "whitespace_in_header", "no_scheme"], ) def test_non_data_uri_text_is_not_flagged(value) -> None: - """The header is bounded to 256 characters, so a 300-character header is not a data URI here.""" - assert _DATA_URI.search(value) is 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_bounded_data_uri_matches_unbounded_pattern_below_the_header_bound() -> None: - """Below the 256-character bound the linear pattern agrees with the old unbounded one.""" +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 bool(_DATA_URI.search(value)) == bool(unbounded.search(value)), value + assert _contains_data_uri(value) == bool(unbounded.search(value)), value