From 71f77f40c6ada879b47488b64cd4153e887f2d0d Mon Sep 17 00:00:00 2001 From: Christopher Kevin Date: Tue, 4 Aug 2026 00:30:06 -0700 Subject: [PATCH 1/6] fix(output-handling): ignore RegExp exec parsing Signed-off-by: Christopher Kevin --- .../static_patterns_output_handling.py | 259 +++++++++++++++++- tests/unit/test_patterns_new.py | 191 +++++++++++++ 2 files changed, 449 insertions(+), 1 deletion(-) diff --git a/src/skillspector/nodes/analyzers/static_patterns_output_handling.py b/src/skillspector/nodes/analyzers/static_patterns_output_handling.py index 1bdbfd3fb..e00f96fc6 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_output_handling.py +++ b/src/skillspector/nodes/analyzers/static_patterns_output_handling.py @@ -70,11 +70,33 @@ """, re.IGNORECASE | re.VERBOSE, ) +_EXEC_OUTPUT_PATTERN = r"exec\s*\(\s*(?:response|output|result|answer|completion|reply|generated)" +_JAVASCRIPT_FILE_TYPES = frozenset({"javascript", "typescript"}) +_JAVASCRIPT_EXTENSIONS = frozenset({".cjs", ".cts", ".js", ".jsx", ".mjs", ".mts", ".ts", ".tsx"}) +_JAVASCRIPT_REGEXP_FLAGS = frozenset("dgimsuvy") +_JAVASCRIPT_REGEXP_LOOKBACK_CHARS = 4_096 +_JAVASCRIPT_LINE_TERMINATORS = "\r\n\u2028\u2029" +_JAVASCRIPT_EXPRESSION_PREFIX_CHARACTERS = frozenset("=([{,:;!?&|+-*%^~<>") +_JAVASCRIPT_EXPRESSION_PREFIX_KEYWORDS = frozenset( + { + "case", + "delete", + "do", + "else", + "in", + "instanceof", + "new", + "return", + "throw", + "typeof", + "void", + } +) # OH1: Unvalidated Output Injection — model output used directly in dangerous sinks OH1_PATTERNS = [ # Python: output piped into exec/eval. Subprocess calls are inspected via AST below. - (r"exec\s*\(\s*(?:response|output|result|answer|completion|reply|generated)", 0.9), + (_EXEC_OUTPUT_PATTERN, 0.9), (r"eval\s*\(\s*(?:response|output|result|answer|completion|reply|generated)", 0.9), (r"os\.system\s*\(\s*(?:response|output|result|answer|completion)", 0.85), (r"os\.popen\s*\(\s*(?:response|output|result|answer|completion)", 0.85), @@ -174,6 +196,237 @@ def _contains_output_name(node: ast.AST) -> bool: return False +def _is_javascript_source(file_path: str, file_type: str) -> bool: + """Return whether analyzer inputs identify JavaScript or TypeScript source.""" + suffix_start = file_path.rfind(".") + suffix = file_path[suffix_start:].casefold() if suffix_start >= 0 else "" + return file_type in _JAVASCRIPT_FILE_TYPES or suffix in _JAVASCRIPT_EXTENSIONS + + +def _skip_javascript_whitespace_backward(content: str, index: int, floor: int) -> int: + """Skip JavaScript whitespace before *index*, but deliberately not comments. + + Recognizing comments without a JavaScript lexer is unsafe because ``/*`` + and ``//`` are both valid text inside regexp character classes. Treating + those sequences as trivia can skip into a preceding regexp and make an + unrelated ``exec(output)`` call look like ``RegExp.prototype.exec``. + Comment-separated receivers therefore fail closed as OH1 findings. + """ + while index > floor and content[index - 1].isspace(): + index -= 1 + return index + + +def _is_javascript_character_escaped(content: str, index: int, floor: int) -> bool: + """Return whether the character at *index* has an odd backslash prefix.""" + backslashes = 0 + cursor = index - 1 + while cursor >= floor and content[cursor] == "\\": + backslashes += 1 + cursor -= 1 + return backslashes % 2 == 1 + + +def _is_javascript_identifier_part(character: str) -> bool: + """Conservatively return whether *character* may continue a JS identifier. + + Python exposes Unicode ``XID_Continue`` through ``str.isidentifier()``, + while JavaScript uses ``ID_Continue`` and can support a newer Unicode + version. Treat an otherwise-unknown non-ASCII character as an identifier + part so version or normalization differences fail closed instead of + splitting an identifier such as ``x\u037areturn`` at the ``return`` suffix. + """ + if character.isspace(): + return False + return ord(character) > 0x7F or character == "$" or ("a" + character).isidentifier() + + +def _javascript_braced_unicode_escape_ends_at(content: str, index: int, floor: int) -> bool: + """Return whether a ``\\u{...}`` escape ends immediately before *index*.""" + if index <= floor or content[index - 1] != "}": + return False + + cursor = index - 2 + while cursor >= floor and content[cursor] in "0123456789abcdefABCDEF": + cursor -= 1 + if cursor == index - 2: + return False + if cursor < floor: + return True + if content[cursor] != "{": + return False + if cursor - 2 < floor: + return True + return content[cursor - 2 : cursor] == "\\u" + + +def _javascript_regexp_opening_has_unambiguous_line_prefix( + content: str, opening_slash: int, floor: int +) -> bool: + """Reject an opening candidate when earlier line syntax makes it ambiguous. + + A slash immediately before ``g.exec`` may be division, and the nearest + preceding slash may then be the *closing* delimiter of another regexp. + Only accept a candidate when there is no earlier code slash on its line. + Slashes inside ordinary quoted strings are ignored; comments and template + literals deliberately fail closed because they need a full JS lexer. + """ + last_line_break = max( + content.rfind(terminator, floor, opening_slash) + for terminator in _JAVASCRIPT_LINE_TERMINATORS + ) + if last_line_break >= floor: + line_start = last_line_break + 1 + elif floor == 0 or content[floor - 1] in _JAVASCRIPT_LINE_TERMINATORS: + line_start = floor + else: + return False + + quote: str | None = None + escaped = False + for character in content[line_start:opening_slash]: + if quote is not None: + if escaped: + escaped = False + elif character == "\\": + escaped = True + elif character == quote: + quote = None + continue + + if character in {'"', "'"}: + quote = character + elif character in {"`", "/"}: + return False + + return quote is None + + +def _find_javascript_regexp_opening_slash( + content: str, closing_slash: int, floor: int +) -> int | None: + """Find a regexp literal's opening slash without mistaking class slashes.""" + in_character_class = False + cursor = closing_slash - 1 + while cursor >= floor: + character = content[cursor] + if character in _JAVASCRIPT_LINE_TERMINATORS: + return None + + if character in "/[]" and not _is_javascript_character_escaped(content, cursor, floor): + if character == "]": + in_character_class = True + elif character == "[" and in_character_class: + in_character_class = False + elif character == "/" and not in_character_class: + return cursor if cursor + 1 < closing_slash else None + cursor -= 1 + return None + + +def _javascript_expression_can_start_at(content: str, index: int, floor: int) -> bool: + """Conservatively validate that a JavaScript expression may start at *index*.""" + cursor = _skip_javascript_whitespace_backward(content, index, floor) + if cursor == floor: + return floor == 0 + + previous = content[cursor - 1] + if previous in _JAVASCRIPT_EXPRESSION_PREFIX_CHARACTERS: + if previous == ">": + return cursor - floor >= 2 and content[cursor - 2] == "=" + if previous in "+-" and cursor - floor >= 2 and content[cursor - 2] == previous: + return False + if previous == "!": + operator_start = cursor - 1 + while True: + prefix_end = _skip_javascript_whitespace_backward(content, operator_start, floor) + if prefix_end <= floor or content[prefix_end - 1] != "!": + break + operator_start = prefix_end - 1 + return _javascript_expression_can_start_at(content, operator_start, floor) + return True + if not _is_javascript_identifier_part(previous): + return False + + token_start = cursor - 1 + while token_start > floor and _is_javascript_identifier_part(content[token_start - 1]): + token_start -= 1 + token = content[token_start:cursor] + if _javascript_braced_unicode_escape_ends_at(content, token_start, floor): + return False + token_prefix = _skip_javascript_whitespace_backward(content, token_start, floor) + if token_prefix == floor and floor > 0: + # The bounded window may start inside an identifier or after a property + # accessor. Without the preceding lexical context, treating a suffix + # such as ``return`` as a keyword could suppress an unrelated sink. + return False + if token_prefix > floor and content[token_prefix - 1] in ".#": + return False + return token in _JAVASCRIPT_EXPRESSION_PREFIX_KEYWORDS + + +def _is_javascript_regexp_literal_exec( + content: str, + match: re.Match[str], + file_path: str, + file_type: str, +) -> bool: + """Return whether an ``exec`` match is called on a JavaScript regexp literal. + + This bounded backward recognizer handles whitespace, optional chaining, + and parentheses around the literal. It deliberately rejects comments and + a parenthesized function argument such as ``makeRunner(/x/).exec(output)``. + Contexts that require matching an earlier control header remain findings. + """ + if not _is_javascript_source(file_path, file_type): + return False + if content[match.start() : match.start() + 4] != "exec": + # The surrounding OH1 pattern is case-insensitive, but JavaScript + # property names are not. Only the built-in lowercase method is safe. + return False + + floor = max(0, match.start() - _JAVASCRIPT_REGEXP_LOOKBACK_CHARS) + cursor = _skip_javascript_whitespace_backward(content, match.start(), floor) + if cursor <= floor or content[cursor - 1] != ".": + return False + cursor = _skip_javascript_whitespace_backward(content, cursor - 1, floor) + + if cursor > floor and content[cursor - 1] == "?": + cursor = _skip_javascript_whitespace_backward(content, cursor - 1, floor) + + closing_parentheses = 0 + while cursor > floor and content[cursor - 1] == ")": + closing_parentheses += 1 + cursor = _skip_javascript_whitespace_backward(content, cursor - 1, floor) + + while cursor > floor and content[cursor - 1] in _JAVASCRIPT_REGEXP_FLAGS: + cursor -= 1 + if cursor <= floor or content[cursor - 1] != "/": + return False + + opening_slash = _find_javascript_regexp_opening_slash(content, cursor - 1, floor) + if opening_slash is None: + return False + if not _javascript_regexp_opening_has_unambiguous_line_prefix(content, opening_slash, floor): + return False + if not _javascript_expression_can_start_at(content, opening_slash, floor): + return False + + wrapper_start = opening_slash + for _ in range(closing_parentheses): + wrapper_start = _skip_javascript_whitespace_backward(content, wrapper_start, floor) + if wrapper_start <= floor or content[wrapper_start - 1] != "(": + return False + wrapper_start -= 1 + + if closing_parentheses and not _javascript_expression_can_start_at( + content, wrapper_start, floor + ): + return False + + return True + + def _subprocess_execution_arguments(node: ast.Call, method_name: str) -> list[ast.expr]: """Return subprocess arguments that can supply executed content.""" execution_keywords = _SUBPROCESS_EXECUTION_KEYWORDS.get(method_name) @@ -275,6 +528,10 @@ def ctx(start: int) -> str: for pattern, confidence in OH1_PATTERNS: for match in re.finditer(pattern, content, re.IGNORECASE | re.MULTILINE): + if pattern == _EXEC_OUTPUT_PATTERN and _is_javascript_regexp_literal_exec( + content, match, file_path, file_type + ): + continue line_num = get_line_number(content, match.start()) adj = ( min(1.0, confidence + 0.1) diff --git a/tests/unit/test_patterns_new.py b/tests/unit/test_patterns_new.py index e765c3041..ae15d021b 100644 --- a/tests/unit/test_patterns_new.py +++ b/tests/unit/test_patterns_new.py @@ -244,6 +244,197 @@ class TestOutputHandling: def test_oh1_detected(self, content: str, filename: str, filetype: str) -> None: assert any(f.rule_id == "OH1" for f in oh_mod.analyze(content, filename, filetype)) + def test_reported_regexp_literal_exec_is_not_output_injection(self) -> None: + content = r"const match = /Process exited with code\s+(-?\d+)/u.exec(output);" + + findings = oh_mod.analyze(content, "scripts/importers/codex.ts", "typescript") + + assert not any(f.rule_id == "OH1" for f in findings) + + @pytest.mark.parametrize( + "content", + [ + pytest.param("return /error/i.exec(output);", id="return_expression"), + pytest.param("const parse = () => /error/i.exec(output);", id="arrow_expression"), + pytest.param( + "const match = condition ? /yes/.exec(output) : null;", + id="conditional_expression", + ), + pytest.param("const match = ((/error/i)).exec(output);", id="parenthesized"), + pytest.param("const match = /error/i\n .exec(output);", id="line_broken"), + pytest.param(r"const match = /[\/]/u.exec(output);", id="character_class_slash"), + pytest.param("const match = (/error/i)?.exec(output);", id="optional_chain"), + pytest.param( + 'const url = "https://example.test"; const match = /error/i.exec(output);', + id="url_string_before_literal", + ), + pytest.param( + 'const url = "https://example.test"; const match = /error/i\n .exec(output);', + id="url_string_before_line_break", + ), + pytest.param("return (/error/i).exec(output);", id="grouped_return"), + pytest.param("throw (/error/i).exec(output);", id="grouped_throw"), + pytest.param("typeof (/error/i).exec(output);", id="grouped_unary_keyword"), + pytest.param("return !/error/i.exec(output);", id="unary_not"), + pytest.param("return\u00a0/error/i.exec(output);", id="unicode_whitespace"), + ], + ) + def test_regexp_literal_exec_is_not_output_injection(self, content: str) -> None: + findings = oh_mod.analyze(content, "parser.ts", "typescript") + + assert not any(f.rule_id == "OH1" for f in findings) + + @pytest.mark.parametrize("filename", ["parser.mjs", "parser.tsx"]) + def test_regexp_literal_exec_recognizes_javascript_family_extensions( + self, filename: str + ) -> None: + findings = oh_mod.analyze("const match = /error/i.exec(output);", filename, "other") + + assert not any(f.rule_id == "OH1" for f in findings) + + @pytest.mark.parametrize( + "content", + [ + pytest.param("child_process.exec(output)", id="child_process"), + pytest.param("child_process .\n exec ( output )", id="child_process_spaced"), + pytest.param("exec(output)", id="imported_exec_alias"), + pytest.param("runner.exec(output)", id="unknown_exec_method"), + pytest.param( + "const ratio = left / right; child_process.exec(output)", id="nearby_division" + ), + pytest.param("left/right/g.exec(output)", id="division_short_receiver"), + pytest.param("left/right/g?.exec(output)", id="division_optional_receiver"), + pytest.param("left++/right/g.exec(output)", id="postfix_increment"), + pytest.param("left--/right/g.exec(output)", id="postfix_decrement"), + pytest.param("left!/right/g.exec(output)", id="non_null_identifier"), + pytest.param('"left"!/right/g.exec(output)', id="non_null_string"), + pytest.param("`left`!/right/g.exec(output)", id="non_null_template"), + pytest.param("/left/!/right/g.exec(output)", id="non_null_regexp"), + pytest.param("left!!!/right/g.exec(output)", id="chained_non_null"), + pytest.param("const z =
/right/g.exec(output)", id="jsx_element"), + pytest.param("fn/right/g.exec(output)", id="typescript_instantiation"), + pytest.param("obj.return/right/g.exec(output)", id="keyword_property"), + pytest.param("obj?.await/right/g.exec(output)", id="optional_keyword_property"), + pytest.param( + "class C { #return = 8; run(right, g, output) { " + "return this.#return/right/g.exec(output); } }", + id="private_keyword_field", + ), + pytest.param("of/right/g.exec(output)", id="contextual_of_identifier"), + pytest.param("await/right/g.exec(output)", id="contextual_await_identifier"), + pytest.param("yield/right/g.exec(output)", id="contextual_yield_identifier"), + pytest.param("x\u200creturn/right/g.exec(output)", id="zwnj_identifier"), + pytest.param("x\u0301return/right/g.exec(output)", id="combining_mark_identifier"), + pytest.param( + "x\u037areturn/right/g.exec(output)", + id="javascript_id_continue_not_python_xid", + ), + pytest.param( + r"x\u{37A}return/right/g.exec(output)", + id="braced_unicode_escape_identifier", + ), + pytest.param( + r"x\u{00000037A}return/right/g.exec(output)", + id="long_braced_unicode_escape_identifier", + ), + pytest.param("makeRunner(/x/).exec(output)", id="call_result_exec"), + pytest.param('"/x/".exec(output)', id="slash_shaped_string"), + pytest.param("/x/.EXEC(output)", id="uppercase_custom_method"), + pytest.param("/x/.Exec(output)", id="mixed_case_custom_method"), + pytest.param( + "return left / /x=/ /g.exec(output);", + id="nested_regexp_closing_slash_before_division", + ), + pytest.param( + "const t = `${left / /x=/ /g.exec(output)}`;", + id="nested_regexp_closing_slash_in_template_expression", + ), + pytest.param( + "return /[/*]*/ /right/g.exec(output);", + id="regexp_block_comment_lookalike_before_division", + ), + pytest.param( + "return /[ //]+/\n/right/g.exec(output);", + id="regexp_line_comment_lookalike_before_division", + ), + pytest.param( + "const r = /[/x/. //]+/;\nexec(output);", + id="regexp_line_comment_lookalike_before_standalone_exec", + ), + pytest.param( + "const r = /[/x/. /*]*/\nexec(output);", + id="regexp_block_comment_lookalike_before_standalone_exec", + ), + ], + ) + def test_dangerous_exec_sinks_remain_output_injection(self, content: str) -> None: + findings = oh_mod.analyze(content, "runner.ts", "typescript") + + assert any(f.rule_id == "OH1" for f in findings) + + @pytest.mark.parametrize( + "content", + [ + pytest.param( + "const match = /error/i /* parsing only */ .exec(output);", + id="block_comment", + ), + pytest.param( + "const match = /error/i // parsing only\n .exec(output);", + id="line_comment", + ), + ], + ) + def test_comment_separated_regexp_exec_fails_closed(self, content: str) -> None: + findings = oh_mod.analyze(content, "parser.ts", "typescript") + + assert any(f.rule_id == "OH1" for f in findings) + + def test_python_exec_remains_output_injection(self) -> None: + findings = oh_mod.analyze("exec(output)", "runner.py", "python") + + assert any(f.rule_id == "OH1" for f in findings) + + def test_regexp_literal_detection_remains_bounded_on_large_files(self) -> None: + suffix = "\nreturn /error/i.exec(output);" + content = ("x" * (1_000_000 - len(suffix))) + suffix + + findings = oh_mod.analyze(content, "parser.ts", "typescript") + + assert not any(f.rule_id == "OH1" for f in findings) + + def test_regexp_literal_detection_fails_closed_at_lookback_boundary(self) -> None: + middle = "a" * (oh_mod._JAVASCRIPT_REGEXP_LOOKBACK_CHARS - 11) + content = f"xreturn /{middle}/g.exec(output);" + + findings = oh_mod.analyze(content, "runner.ts", "typescript") + + assert any(f.rule_id == "OH1" for f in findings) + + def test_braced_unicode_identifier_escape_fails_closed_at_lookback_boundary( + self, + ) -> None: + zeros = "0" * (oh_mod._JAVASCRIPT_REGEXP_LOOKBACK_CHARS + 1) + content = rf"x\u{{{zeros}37A}}return/right/g.exec(output)" + + findings = oh_mod.analyze(content, "runner.ts", "typescript") + + assert any(f.rule_id == "OH1" for f in findings) + + def test_regexp_literal_detection_scans_escape_runs_linearly(self) -> None: + regexp = "/" + ("\\" * 3_500) + "x/" + content = "\n".join(f"const match{index} = {regexp}.exec(output);" for index in range(10)) + + with patch.object( + oh_mod, + "_is_javascript_character_escaped", + wraps=oh_mod._is_javascript_character_escaped, + ) as escape_check: + findings = oh_mod.analyze(content, "parser.ts", "typescript") + + assert not any(f.rule_id == "OH1" for f in findings) + assert escape_check.call_count <= 30 + def test_oh1_confidence_boost_for_python(self) -> None: findings = oh_mod.analyze('exec(response["code"])', "runner.py", "python") oh1 = [f for f in findings if f.rule_id == "OH1"] From 8f8a104d841da71d511a0a0eeac7893f2459659d Mon Sep 17 00:00:00 2001 From: Christopher Kevin Date: Tue, 4 Aug 2026 03:50:12 -0700 Subject: [PATCH 2/6] fix(output-handling): guard RegExp exec mutations Signed-off-by: Christopher Kevin --- .../static_patterns_output_handling.py | 315 +++++++++++++++++- tests/unit/test_patterns_new.py | 200 +++++++++++ 2 files changed, 510 insertions(+), 5 deletions(-) diff --git a/src/skillspector/nodes/analyzers/static_patterns_output_handling.py b/src/skillspector/nodes/analyzers/static_patterns_output_handling.py index e00f96fc6..e5b553f08 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_output_handling.py +++ b/src/skillspector/nodes/analyzers/static_patterns_output_handling.py @@ -26,7 +26,6 @@ import ast import re -import sys from skillspector.logging_config import get_logger from skillspector.models import AnalyzerFinding, Location, Severity @@ -92,6 +91,47 @@ "void", } ) +_JAVASCRIPT_MUTATION_TRIVIA = r"(?:\s|/\*(?:[^*]|\*(?!/))*\*/|//[^\r\n]*(?:\r\n?|\n|$))*" +_JAVASCRIPT_ASSIGNMENT_OPERATOR = r"(?:\?\?=|&&=|\|\|=|\*\*=|>>>=|>>=|<<=|[+\-*/%&|^]?=(?!=|>))" +_JAVASCRIPT_IDENTIFIER = r"(?:[$_]|[^\W\d])[\w$]*" +_JAVASCRIPT_REGEXP_LITERAL = r"/(?:\\[^\r\n]|[^/\\\r\n]){1,4096}/[dgimsuvy]*" +_JAVASCRIPT_REGEXP_CONSTRUCTOR = r"(?:new\s+)?\bRegExp\s*\([^\r\n)]{0,4096}\)" +_JAVASCRIPT_REGEXP_INSTANCE = rf"(?:{_JAVASCRIPT_REGEXP_LITERAL}|{_JAVASCRIPT_REGEXP_CONSTRUCTOR})" +_JAVASCRIPT_REGEXP_PROTOTYPE = r"\bRegExp\s*(?:\.\s*prototype\b|\[\s*['\"`]prototype['\"`]\s*\])" +_JAVASCRIPT_REGEXP_PROTOTYPE_FROM_INSTANCE = ( + rf"(?:\b(?:Object|Reflect)\s*\.\s*getPrototypeOf\s*\(\s*" + rf"(?:\(\s*)*{_JAVASCRIPT_REGEXP_INSTANCE}(?:\s*\))*\s*\)|" + rf"(?:\(\s*)*{_JAVASCRIPT_REGEXP_INSTANCE}(?:\s*\))*\s*\.\s*" + r"(?:__proto__\b|constructor\s*\.\s*prototype\b))" +) +_JAVASCRIPT_REGEXP_PROTOTYPE_EXPRESSION = ( + rf"(?:{_JAVASCRIPT_REGEXP_PROTOTYPE}|" + rf"{_JAVASCRIPT_REGEXP_PROTOTYPE_FROM_INSTANCE})" +) +_JAVASCRIPT_UNICODE_ESCAPE_PATTERN = re.compile( + r"\\(?:u(?:\{(?P[0-9a-fA-F]+)\}|(?P[0-9a-fA-F]{4}))|" + r"x(?P[0-9a-fA-F]{2}))" +) +_JAVASCRIPT_SIMPLE_STRING_CONCAT_PATTERN = re.compile( + r"(?P['\"])(?P[A-Za-z_$]+)(?P=left_quote)\s*\+\s*" + r"(?P['\"])(?P[A-Za-z_$]+)(?P=right_quote)" +) +_JAVASCRIPT_COMMENT_PATTERN = re.compile(r"/\*[\s\S]*?\*/|//[^\r\n]*") +_JAVASCRIPT_EXEC_PROPERTY_ALIAS_PATTERN = re.compile( + rf"\b(?:const|let|var)\s+(?P{_JAVASCRIPT_IDENTIFIER})\s*=\s*['\"`]exec['\"`]" +) +_JAVASCRIPT_REGEXP_INSTANCE_ALIAS_PATTERN = re.compile( + rf"\b(?:const|let|var)\s+(?P{_JAVASCRIPT_IDENTIFIER})\s*=\s*" + rf"{_JAVASCRIPT_REGEXP_INSTANCE}(?=\s*(?:[;,\r\n]|$))" +) +_JAVASCRIPT_IDENTIFIER_ALIAS_PATTERN = re.compile( + rf"\b(?:const|let|var)\s+(?P{_JAVASCRIPT_IDENTIFIER})\s*=\s*" + rf"(?P{_JAVASCRIPT_IDENTIFIER})\b(?=\s*(?:[;,\r\n]|$))" +) +_JAVASCRIPT_IMPORT_ALIAS_PATTERN = re.compile( + rf"\b(?P{_JAVASCRIPT_IDENTIFIER})\s+as\s+" + rf"(?P{_JAVASCRIPT_IDENTIFIER})\b" +) # OH1: Unvalidated Output Injection — model output used directly in dangerous sinks OH1_PATTERNS = [ @@ -203,6 +243,238 @@ def _is_javascript_source(file_path: str, file_type: str) -> bool: return file_type in _JAVASCRIPT_FILE_TYPES or suffix in _JAVASCRIPT_EXTENSIONS +def _javascript_unicode_escape_value(match: re.Match[str]) -> str: + """Decode a bounded JavaScript Unicode or hexadecimal escape for scanning.""" + digits = match.group("braced") or match.group("fixed") or match.group("hex") + significant = digits.lstrip("0") or "0" + if len(significant) > 6: + return match.group(0) + value = int(significant, 16) + if value > 0x10FFFF or 0xD800 <= value <= 0xDFFF: + return match.group(0) + return chr(value) + + +def _javascript_mutation_scan_variants(content: str) -> tuple[str, ...]: + """Return conservative source variants for mutation signal matching. + + The raw variant prevents comment-like text inside regexp literals from + hiding later code. The comment-collapsed variant recognizes mutations with + comments inserted between JavaScript tokens. Identifier and string escapes + plus simple constant string concatenations are normalized in both. + """ + normalized = _JAVASCRIPT_UNICODE_ESCAPE_PATTERN.sub(_javascript_unicode_escape_value, content) + for _ in range(4): + folded = _JAVASCRIPT_SIMPLE_STRING_CONCAT_PATTERN.sub( + lambda match: f'"{match.group("left")}{match.group("right")}"', + normalized, + ) + if folded == normalized: + break + normalized = folded + without_comments = _JAVASCRIPT_COMMENT_PATTERN.sub(" ", normalized) + return (normalized,) if without_comments == normalized else (normalized, without_comments) + + +def _javascript_expand_aliases( + seeds: set[str], + contents: tuple[str, ...], + *, + include_imports: bool = True, +) -> frozenset[str]: + """Propagate simple identifier and import aliases from known seed names.""" + aliases = set(seeds) + aliases_by_source: dict[str, set[str]] = {} + for content in contents: + patterns = [_JAVASCRIPT_IDENTIFIER_ALIAS_PATTERN] + if include_imports: + patterns.append(_JAVASCRIPT_IMPORT_ALIAS_PATTERN) + for pattern in patterns: + for match in pattern.finditer(content): + aliases_by_source.setdefault(match.group("source"), set()).add( + match.group("target") + ) + + pending = list(aliases) + while pending: + source = pending.pop() + for target in aliases_by_source.get(source, ()): + if target not in aliases: + aliases.add(target) + pending.append(target) + return frozenset(aliases) + + +def _javascript_exec_property_aliases(contents: tuple[str, ...]) -> frozenset[str]: + """Collect simple local or imported aliases whose constant value is ``exec``.""" + seeds = { + match.group("name") + for content in contents + for match in _JAVASCRIPT_EXEC_PROPERTY_ALIAS_PATTERN.finditer(content) + } + return _javascript_expand_aliases(seeds, contents) + + +def _javascript_regexp_prototype_receiver( + contents: tuple[str, ...], +) -> str: + """Build a pattern for direct and simply aliased RegExp prototype receivers.""" + constructor_aliases = _javascript_expand_aliases( + {"RegExp"}, contents, include_imports=False + ) - {"RegExp"} + needs_instance_aliases = any( + any(hint in content for hint in ("getPrototypeOf", "__proto__", "constructor")) + for content in contents + ) + instance_seeds = ( + { + match.group("target") + for content in contents + for match in _JAVASCRIPT_REGEXP_INSTANCE_ALIAS_PATTERN.finditer(content) + } + if needs_instance_aliases + else set() + ) + instance_aliases = _javascript_expand_aliases(instance_seeds, contents) + + prototype_expressions = [_JAVASCRIPT_REGEXP_PROTOTYPE_EXPRESSION] + if constructor_aliases: + constructors = "|".join(re.escape(alias) for alias in sorted(constructor_aliases)) + prototype_expressions.append( + rf"\b(?:{constructors})\b\s*(?:\.\s*prototype\b|" + r"\[\s*['\"`]prototype['\"`]\s*\])" + ) + if instance_aliases: + instances = "|".join(re.escape(alias) for alias in sorted(instance_aliases)) + instance = rf"\b(?:{instances})\b" + prototype_expressions.extend( + ( + rf"\b(?:Object|Reflect)\s*\.\s*getPrototypeOf\s*\(\s*{instance}\s*\)", + rf"{instance}\s*\.\s*(?:__proto__\b|constructor\s*\.\s*prototype\b)", + ) + ) + prototype_expression = rf"(?:{'|'.join(prototype_expressions)})" + + prototype_alias_pattern = re.compile( + rf"\b(?:const|let|var)\s+(?P{_JAVASCRIPT_IDENTIFIER})\s*=\s*" + rf"{prototype_expression}(?=\s*(?:[;,\r\n]|$))" + ) + prototype_seeds = { + match.group("target") + for content in contents + for match in prototype_alias_pattern.finditer(content) + } + prototype_aliases = _javascript_expand_aliases(prototype_seeds, contents) + receivers = [prototype_expression] + if prototype_aliases: + aliases = "|".join(re.escape(alias) for alias in sorted(prototype_aliases)) + receivers.append(rf"\b(?:{aliases})\b") + return rf"(?:{'|'.join(receivers)})" + + +def _javascript_mutates_regexp_exec( + content: str, + receiver: str, + property_aliases: frozenset[str], +) -> bool: + """Return whether *content* mutates ``exec`` on a known RegExp prototype.""" + property_keys = [r"['\"`]exec['\"`]"] + computed_properties: list[str] = [] + if property_aliases: + aliases = "|".join(re.escape(alias) for alias in sorted(property_aliases)) + property_keys.append(rf"\b(?:{aliases})\b") + computed_properties.append(rf"\[\s*(?:{aliases})\s*\]") + property_key = rf"(?:{'|'.join(property_keys)})" + member = r"(?:\.\s*exec\b|\[\s*['\"`]exec['\"`]\s*\]" + if computed_properties: + member += rf"|{'|'.join(computed_properties)}" + member += ")" + wrapped_receiver = rf"(?:\(\s*)*{receiver}(?:\s*\))*" + + patterns = ( + rf"{wrapped_receiver}{_JAVASCRIPT_MUTATION_TRIVIA}{member}" + rf"{_JAVASCRIPT_MUTATION_TRIVIA}{_JAVASCRIPT_ASSIGNMENT_OPERATOR}", + rf"\bdelete{_JAVASCRIPT_MUTATION_TRIVIA}{wrapped_receiver}" + rf"{_JAVASCRIPT_MUTATION_TRIVIA}{member}", + rf"\b(?:Object|Reflect)\s*\.\s*(?:defineProperty|set)\s*\(\s*" + rf"{wrapped_receiver}\s*,\s*{property_key}\s*,", + rf"\bObject\s*\.\s*(?:assign|defineProperties)\s*\(\s*" + rf"{wrapped_receiver}\s*,\s*\{{[^}}\r\n]{{0,4096}}" + rf"(?:\bexec\s*:|['\"`]exec['\"`]\s*:" + rf"|\[\s*{property_key}\s*\]\s*:)", + rf"{wrapped_receiver}{_JAVASCRIPT_MUTATION_TRIVIA}\.\s*" + rf"__define(?:Getter|Setter)__\s*\(\s*{property_key}\s*,", + ) + return any(re.search(pattern, content, re.MULTILINE) for pattern in patterns) + + +def _javascript_regexp_exec_mutation_possible(contents: tuple[str, ...]) -> bool: + """Return whether visible code may replace the method used by regexp literals. + + Prototype acquisition and mutation may occur in different components, so + the graph node evaluates these signals across the complete scanned skill. + The public ``analyze`` helper applies the same rule within a single source. + """ + if not contents: + return False + if not any( + any( + hint in content + for hint in ("RegExp", "getPrototypeOf", "__proto__", "constructor", "\\u", "\\x") + ) + for content in contents + ): + return False + if not any( + any( + hint in content + for hint in ( + "=", + "delete", + "defineProperty", + "defineProperties", + "assign", + "set", + "__define", + ) + ) + for content in contents + ): + return False + + scan_contents = tuple( + dict.fromkeys( + variant + for content in contents + for variant in _javascript_mutation_scan_variants(content) + ) + ) + property_aliases = _javascript_exec_property_aliases(scan_contents) + receiver = _javascript_regexp_prototype_receiver(scan_contents) + return any( + _javascript_mutates_regexp_exec(content, receiver, property_aliases) + for content in scan_contents + ) + + +class _OutputHandlingPatternAdapter: + """Bind whole-skill mutation context to the generic static runner contract.""" + + ANALYZER_ID = "static_patterns_output_handling" + + def __init__(self, regexp_exec_mutation_possible: bool) -> None: + self._regexp_exec_mutation_possible = regexp_exec_mutation_possible + + def analyze(self, *, content: str, file_path: str, file_type: str) -> list[AnalyzerFinding]: + """Analyze one component with whole-skill RegExp mutation context.""" + return analyze( + content, + file_path, + file_type, + regexp_exec_mutation_possible=self._regexp_exec_mutation_possible, + ) + + def _skip_javascript_whitespace_backward(content: str, index: int, floor: int) -> int: """Skip JavaScript whitespace before *index*, but deliberately not comments. @@ -514,10 +786,21 @@ def _analyze_python_subprocess_calls( return findings -def analyze(content: str, file_path: str, file_type: str) -> list[AnalyzerFinding]: +def analyze( + content: str, + file_path: str, + file_type: str, + *, + regexp_exec_mutation_possible: bool | None = None, +) -> list[AnalyzerFinding]: """Analyze content for output handling patterns (OH1–OH3).""" findings: list[AnalyzerFinding] = [] + if regexp_exec_mutation_possible is None: + regexp_exec_mutation_possible = _is_javascript_source( + file_path, file_type + ) and _javascript_regexp_exec_mutation_possible((content,)) + def loc(ln: int) -> Location: return Location(file=file_path, start_line=ln) @@ -528,8 +811,10 @@ def ctx(start: int) -> str: for pattern, confidence in OH1_PATTERNS: for match in re.finditer(pattern, content, re.IGNORECASE | re.MULTILINE): - if pattern == _EXEC_OUTPUT_PATTERN and _is_javascript_regexp_literal_exec( - content, match, file_path, file_type + if ( + pattern == _EXEC_OUTPUT_PATTERN + and not regexp_exec_mutation_possible + and _is_javascript_regexp_literal_exec(content, match, file_path, file_type) ): continue line_num = get_line_number(content, match.start()) @@ -593,6 +878,26 @@ def ctx(start: int) -> str: def node(state: SkillspectorState) -> AnalyzerNodeResponse: """Run output_handling patterns and return findings.""" - response = static_runner.run_static_patterns_with_ledger(state, [sys.modules[__name__]]) + components: list[str] = state.get("components") or [] + file_cache: dict[str, str] = state.get("file_cache") or {} + javascript_contents: list[str] = [] + has_uninspected_javascript = False + for path in components: + if not _is_javascript_source(path, ""): + continue + content = file_cache.get(path) + if ( + content is None + or len(content) > static_runner.MAX_FILE_CHARS + or "\x00" in content[:512] + ): + has_uninspected_javascript = True + else: + javascript_contents.append(content) + adapter = _OutputHandlingPatternAdapter( + has_uninspected_javascript + or _javascript_regexp_exec_mutation_possible(tuple(javascript_contents)) + ) + response = static_runner.run_static_patterns_with_ledger(state, [adapter]) logger.info("%s: %d findings", ANALYZER_ID, len(response["findings"])) return response diff --git a/tests/unit/test_patterns_new.py b/tests/unit/test_patterns_new.py index ae15d021b..42d2ffe92 100644 --- a/tests/unit/test_patterns_new.py +++ b/tests/unit/test_patterns_new.py @@ -390,6 +390,206 @@ def test_comment_separated_regexp_exec_fails_closed(self, content: str) -> None: assert any(f.rule_id == "OH1" for f in findings) + @pytest.mark.parametrize( + "mutation", + [ + pytest.param("RegExp.prototype.exec = eval;", id="direct_assignment"), + pytest.param('RegExp.prototype["exec"] = eval;', id="bracket_assignment"), + pytest.param("RegExp.prototype[`exec`] = eval;", id="template_key_assignment"), + pytest.param( + 'Object.defineProperty(RegExp.prototype, "exec", { value: eval });', + id="define_property", + ), + pytest.param( + 'Reflect.defineProperty(RegExp.prototype, "exec", { value: eval });', + id="reflect_define_property", + ), + pytest.param( + "Object.defineProperties(RegExp.prototype, { exec: { value: eval } });", + id="define_properties", + ), + pytest.param( + 'Reflect.set(RegExp.prototype, "exec", eval);', + id="reflect_set", + ), + pytest.param( + "Object.assign(RegExp.prototype, { exec: eval });", + id="object_assign", + ), + pytest.param( + "const regexPrototype = RegExp.prototype; regexPrototype.exec = eval;", + id="prototype_alias", + ), + pytest.param( + "const π = RegExp.prototype; π.exec = eval;", + id="unicode_prototype_alias", + ), + pytest.param( + "const NativeRegExp = RegExp; NativeRegExp.prototype.exec = eval;", + id="constructor_alias", + ), + pytest.param( + "const regexPrototype = Object.getPrototypeOf(/x/); regexPrototype.exec = eval;", + id="get_prototype_alias", + ), + pytest.param( + "const regexPrototype = /x/.__proto__; regexPrototype.exec = eval;", + id="legacy_proto_alias", + ), + pytest.param( + "const regex = /x/; regex.__proto__.exec = eval;", + id="regexp_instance_alias", + ), + pytest.param( + "const regexPrototype = Object.getPrototypeOf(new RegExp()); " + "regexPrototype.exec = eval;", + id="constructor_instance_alias", + ), + pytest.param( + "Object.getPrototypeOf(/x/).exec = eval;", + id="direct_get_prototype", + ), + pytest.param( + "(/x/).constructor.prototype.exec = eval;", + id="literal_constructor_prototype", + ), + pytest.param( + "Object.prototype.exec = eval; delete RegExp.prototype.exec;", + id="delete_to_inherited_exec", + ), + pytest.param( + "RegExp /* gap */ . /* gap */ prototype . /* gap */ exec /* gap */ = eval;", + id="comment_separated_assignment", + ), + pytest.param( + r"RegExp.prot\u006ftype.ex\u0065c = eval;", + id="identifier_unicode_escapes", + ), + pytest.param( + r'RegExp["prot\u006ftype"]["ex\u0065c"] = eval;', + id="string_unicode_escapes", + ), + pytest.param( + 'const property = "exec"; RegExp.prototype[property] = eval;', + id="computed_property_alias", + ), + pytest.param( + 'const property = "ex" + "ec"; RegExp.prototype[property] = eval;', + id="concatenated_property_alias", + ), + pytest.param( + 'Object.defineProperty(RegExp.prototype, "ex" + "ec", { value: eval });', + id="computed_define_property", + ), + pytest.param( + 'RegExp.prototype.__defineGetter__("exec", () => eval);', + id="legacy_define_getter", + ), + ], + ) + def test_regexp_literal_exec_fails_closed_when_prototype_may_be_mutated( + self, mutation: str + ) -> None: + content = f'{mutation}\nconst output = "globalThis.compromised = true";\n/x/.exec(output);' + + findings = oh_mod.analyze(content, "attack.js", "javascript") + + assert any(f.rule_id == "OH1" for f in findings) + + def test_regexp_exec_prototype_mutation_fails_closed_across_files(self) -> None: + response = oh_mod.node( + { + "components": ["prototype.js", "mutation.js", "parser.js"], + "file_cache": { + "prototype.js": "export const regexPrototype = RegExp.prototype;", + "mutation.js": ( + 'import { regexPrototype } from "./prototype.js";\n' + "regexPrototype.exec = eval;" + ), + "parser.js": ( + 'const output = "globalThis.compromised = true";\n/x/.exec(output);' + ), + }, + } + ) + + assert any( + finding.rule_id == "OH1" and finding.file == "parser.js" + for finding in response["findings"] + ) + + def test_unrelated_exec_assignment_does_not_disable_regexp_literal_exemption( + self, + ) -> None: + response = oh_mod.node( + { + "components": ["runner.js", "parser.js"], + "file_cache": { + "runner.js": "runner.exec = handler;", + "parser.js": "const match = /x/.exec(output);", + }, + } + ) + + assert not any(finding.rule_id == "OH1" for finding in response["findings"]) + + def test_reading_regexp_prototype_does_not_disable_literal_exemption(self) -> None: + response = oh_mod.node( + { + "components": ["introspection.js", "parser.js"], + "file_cache": { + "introspection.js": "const descriptor = RegExp.prototype.exec;", + "parser.js": "const match = /x/.exec(output);", + }, + } + ) + + assert not any(finding.rule_id == "OH1" for finding in response["findings"]) + + def test_unrelated_prototype_read_and_exec_write_do_not_combine(self) -> None: + response = oh_mod.node( + { + "components": ["introspection.js", "runner.js", "parser.js"], + "file_cache": { + "introspection.js": "const descriptor = RegExp.prototype.exec;", + "runner.js": "runner.exec = handler;", + "parser.js": "const match = /x/.exec(output);", + }, + } + ) + + assert not any(finding.rule_id == "OH1" for finding in response["findings"]) + + @pytest.mark.parametrize( + "uninspected_content", + [ + pytest.param(None, id="missing_cache_entry"), + pytest.param("\x00unknown", id="binary_content"), + pytest.param( + "x" * (oh_mod.static_runner.MAX_FILE_CHARS + 1), + id="over_size_limit", + ), + ], + ) + def test_uninspected_javascript_sibling_fails_closed( + self, uninspected_content: str | None + ) -> None: + file_cache = {"parser.js": "const match = /x/.exec(output);"} + if uninspected_content is not None: + file_cache["unknown.js"] = uninspected_content + + response = oh_mod.node( + { + "components": ["unknown.js", "parser.js"], + "file_cache": file_cache, + } + ) + + assert any( + finding.rule_id == "OH1" and finding.file == "parser.js" + for finding in response["findings"] + ) + def test_python_exec_remains_output_injection(self) -> None: findings = oh_mod.analyze("exec(output)", "runner.py", "python") From 6a155bf1dfab5fc113a1977579d735105736fb5e Mon Sep 17 00:00:00 2001 From: Christopher Kevin Date: Tue, 4 Aug 2026 04:11:34 -0700 Subject: [PATCH 3/6] fix(output-handling): avoid unverified mutation inference Signed-off-by: Christopher Kevin --- .../static_patterns_output_handling.py | 321 +----------------- tests/unit/test_patterns_new.py | 191 ++--------- 2 files changed, 30 insertions(+), 482 deletions(-) diff --git a/src/skillspector/nodes/analyzers/static_patterns_output_handling.py b/src/skillspector/nodes/analyzers/static_patterns_output_handling.py index e5b553f08..09535451c 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_output_handling.py +++ b/src/skillspector/nodes/analyzers/static_patterns_output_handling.py @@ -26,6 +26,7 @@ import ast import re +import sys from skillspector.logging_config import get_logger from skillspector.models import AnalyzerFinding, Location, Severity @@ -91,47 +92,6 @@ "void", } ) -_JAVASCRIPT_MUTATION_TRIVIA = r"(?:\s|/\*(?:[^*]|\*(?!/))*\*/|//[^\r\n]*(?:\r\n?|\n|$))*" -_JAVASCRIPT_ASSIGNMENT_OPERATOR = r"(?:\?\?=|&&=|\|\|=|\*\*=|>>>=|>>=|<<=|[+\-*/%&|^]?=(?!=|>))" -_JAVASCRIPT_IDENTIFIER = r"(?:[$_]|[^\W\d])[\w$]*" -_JAVASCRIPT_REGEXP_LITERAL = r"/(?:\\[^\r\n]|[^/\\\r\n]){1,4096}/[dgimsuvy]*" -_JAVASCRIPT_REGEXP_CONSTRUCTOR = r"(?:new\s+)?\bRegExp\s*\([^\r\n)]{0,4096}\)" -_JAVASCRIPT_REGEXP_INSTANCE = rf"(?:{_JAVASCRIPT_REGEXP_LITERAL}|{_JAVASCRIPT_REGEXP_CONSTRUCTOR})" -_JAVASCRIPT_REGEXP_PROTOTYPE = r"\bRegExp\s*(?:\.\s*prototype\b|\[\s*['\"`]prototype['\"`]\s*\])" -_JAVASCRIPT_REGEXP_PROTOTYPE_FROM_INSTANCE = ( - rf"(?:\b(?:Object|Reflect)\s*\.\s*getPrototypeOf\s*\(\s*" - rf"(?:\(\s*)*{_JAVASCRIPT_REGEXP_INSTANCE}(?:\s*\))*\s*\)|" - rf"(?:\(\s*)*{_JAVASCRIPT_REGEXP_INSTANCE}(?:\s*\))*\s*\.\s*" - r"(?:__proto__\b|constructor\s*\.\s*prototype\b))" -) -_JAVASCRIPT_REGEXP_PROTOTYPE_EXPRESSION = ( - rf"(?:{_JAVASCRIPT_REGEXP_PROTOTYPE}|" - rf"{_JAVASCRIPT_REGEXP_PROTOTYPE_FROM_INSTANCE})" -) -_JAVASCRIPT_UNICODE_ESCAPE_PATTERN = re.compile( - r"\\(?:u(?:\{(?P[0-9a-fA-F]+)\}|(?P[0-9a-fA-F]{4}))|" - r"x(?P[0-9a-fA-F]{2}))" -) -_JAVASCRIPT_SIMPLE_STRING_CONCAT_PATTERN = re.compile( - r"(?P['\"])(?P[A-Za-z_$]+)(?P=left_quote)\s*\+\s*" - r"(?P['\"])(?P[A-Za-z_$]+)(?P=right_quote)" -) -_JAVASCRIPT_COMMENT_PATTERN = re.compile(r"/\*[\s\S]*?\*/|//[^\r\n]*") -_JAVASCRIPT_EXEC_PROPERTY_ALIAS_PATTERN = re.compile( - rf"\b(?:const|let|var)\s+(?P{_JAVASCRIPT_IDENTIFIER})\s*=\s*['\"`]exec['\"`]" -) -_JAVASCRIPT_REGEXP_INSTANCE_ALIAS_PATTERN = re.compile( - rf"\b(?:const|let|var)\s+(?P{_JAVASCRIPT_IDENTIFIER})\s*=\s*" - rf"{_JAVASCRIPT_REGEXP_INSTANCE}(?=\s*(?:[;,\r\n]|$))" -) -_JAVASCRIPT_IDENTIFIER_ALIAS_PATTERN = re.compile( - rf"\b(?:const|let|var)\s+(?P{_JAVASCRIPT_IDENTIFIER})\s*=\s*" - rf"(?P{_JAVASCRIPT_IDENTIFIER})\b(?=\s*(?:[;,\r\n]|$))" -) -_JAVASCRIPT_IMPORT_ALIAS_PATTERN = re.compile( - rf"\b(?P{_JAVASCRIPT_IDENTIFIER})\s+as\s+" - rf"(?P{_JAVASCRIPT_IDENTIFIER})\b" -) # OH1: Unvalidated Output Injection — model output used directly in dangerous sinks OH1_PATTERNS = [ @@ -243,238 +203,6 @@ def _is_javascript_source(file_path: str, file_type: str) -> bool: return file_type in _JAVASCRIPT_FILE_TYPES or suffix in _JAVASCRIPT_EXTENSIONS -def _javascript_unicode_escape_value(match: re.Match[str]) -> str: - """Decode a bounded JavaScript Unicode or hexadecimal escape for scanning.""" - digits = match.group("braced") or match.group("fixed") or match.group("hex") - significant = digits.lstrip("0") or "0" - if len(significant) > 6: - return match.group(0) - value = int(significant, 16) - if value > 0x10FFFF or 0xD800 <= value <= 0xDFFF: - return match.group(0) - return chr(value) - - -def _javascript_mutation_scan_variants(content: str) -> tuple[str, ...]: - """Return conservative source variants for mutation signal matching. - - The raw variant prevents comment-like text inside regexp literals from - hiding later code. The comment-collapsed variant recognizes mutations with - comments inserted between JavaScript tokens. Identifier and string escapes - plus simple constant string concatenations are normalized in both. - """ - normalized = _JAVASCRIPT_UNICODE_ESCAPE_PATTERN.sub(_javascript_unicode_escape_value, content) - for _ in range(4): - folded = _JAVASCRIPT_SIMPLE_STRING_CONCAT_PATTERN.sub( - lambda match: f'"{match.group("left")}{match.group("right")}"', - normalized, - ) - if folded == normalized: - break - normalized = folded - without_comments = _JAVASCRIPT_COMMENT_PATTERN.sub(" ", normalized) - return (normalized,) if without_comments == normalized else (normalized, without_comments) - - -def _javascript_expand_aliases( - seeds: set[str], - contents: tuple[str, ...], - *, - include_imports: bool = True, -) -> frozenset[str]: - """Propagate simple identifier and import aliases from known seed names.""" - aliases = set(seeds) - aliases_by_source: dict[str, set[str]] = {} - for content in contents: - patterns = [_JAVASCRIPT_IDENTIFIER_ALIAS_PATTERN] - if include_imports: - patterns.append(_JAVASCRIPT_IMPORT_ALIAS_PATTERN) - for pattern in patterns: - for match in pattern.finditer(content): - aliases_by_source.setdefault(match.group("source"), set()).add( - match.group("target") - ) - - pending = list(aliases) - while pending: - source = pending.pop() - for target in aliases_by_source.get(source, ()): - if target not in aliases: - aliases.add(target) - pending.append(target) - return frozenset(aliases) - - -def _javascript_exec_property_aliases(contents: tuple[str, ...]) -> frozenset[str]: - """Collect simple local or imported aliases whose constant value is ``exec``.""" - seeds = { - match.group("name") - for content in contents - for match in _JAVASCRIPT_EXEC_PROPERTY_ALIAS_PATTERN.finditer(content) - } - return _javascript_expand_aliases(seeds, contents) - - -def _javascript_regexp_prototype_receiver( - contents: tuple[str, ...], -) -> str: - """Build a pattern for direct and simply aliased RegExp prototype receivers.""" - constructor_aliases = _javascript_expand_aliases( - {"RegExp"}, contents, include_imports=False - ) - {"RegExp"} - needs_instance_aliases = any( - any(hint in content for hint in ("getPrototypeOf", "__proto__", "constructor")) - for content in contents - ) - instance_seeds = ( - { - match.group("target") - for content in contents - for match in _JAVASCRIPT_REGEXP_INSTANCE_ALIAS_PATTERN.finditer(content) - } - if needs_instance_aliases - else set() - ) - instance_aliases = _javascript_expand_aliases(instance_seeds, contents) - - prototype_expressions = [_JAVASCRIPT_REGEXP_PROTOTYPE_EXPRESSION] - if constructor_aliases: - constructors = "|".join(re.escape(alias) for alias in sorted(constructor_aliases)) - prototype_expressions.append( - rf"\b(?:{constructors})\b\s*(?:\.\s*prototype\b|" - r"\[\s*['\"`]prototype['\"`]\s*\])" - ) - if instance_aliases: - instances = "|".join(re.escape(alias) for alias in sorted(instance_aliases)) - instance = rf"\b(?:{instances})\b" - prototype_expressions.extend( - ( - rf"\b(?:Object|Reflect)\s*\.\s*getPrototypeOf\s*\(\s*{instance}\s*\)", - rf"{instance}\s*\.\s*(?:__proto__\b|constructor\s*\.\s*prototype\b)", - ) - ) - prototype_expression = rf"(?:{'|'.join(prototype_expressions)})" - - prototype_alias_pattern = re.compile( - rf"\b(?:const|let|var)\s+(?P{_JAVASCRIPT_IDENTIFIER})\s*=\s*" - rf"{prototype_expression}(?=\s*(?:[;,\r\n]|$))" - ) - prototype_seeds = { - match.group("target") - for content in contents - for match in prototype_alias_pattern.finditer(content) - } - prototype_aliases = _javascript_expand_aliases(prototype_seeds, contents) - receivers = [prototype_expression] - if prototype_aliases: - aliases = "|".join(re.escape(alias) for alias in sorted(prototype_aliases)) - receivers.append(rf"\b(?:{aliases})\b") - return rf"(?:{'|'.join(receivers)})" - - -def _javascript_mutates_regexp_exec( - content: str, - receiver: str, - property_aliases: frozenset[str], -) -> bool: - """Return whether *content* mutates ``exec`` on a known RegExp prototype.""" - property_keys = [r"['\"`]exec['\"`]"] - computed_properties: list[str] = [] - if property_aliases: - aliases = "|".join(re.escape(alias) for alias in sorted(property_aliases)) - property_keys.append(rf"\b(?:{aliases})\b") - computed_properties.append(rf"\[\s*(?:{aliases})\s*\]") - property_key = rf"(?:{'|'.join(property_keys)})" - member = r"(?:\.\s*exec\b|\[\s*['\"`]exec['\"`]\s*\]" - if computed_properties: - member += rf"|{'|'.join(computed_properties)}" - member += ")" - wrapped_receiver = rf"(?:\(\s*)*{receiver}(?:\s*\))*" - - patterns = ( - rf"{wrapped_receiver}{_JAVASCRIPT_MUTATION_TRIVIA}{member}" - rf"{_JAVASCRIPT_MUTATION_TRIVIA}{_JAVASCRIPT_ASSIGNMENT_OPERATOR}", - rf"\bdelete{_JAVASCRIPT_MUTATION_TRIVIA}{wrapped_receiver}" - rf"{_JAVASCRIPT_MUTATION_TRIVIA}{member}", - rf"\b(?:Object|Reflect)\s*\.\s*(?:defineProperty|set)\s*\(\s*" - rf"{wrapped_receiver}\s*,\s*{property_key}\s*,", - rf"\bObject\s*\.\s*(?:assign|defineProperties)\s*\(\s*" - rf"{wrapped_receiver}\s*,\s*\{{[^}}\r\n]{{0,4096}}" - rf"(?:\bexec\s*:|['\"`]exec['\"`]\s*:" - rf"|\[\s*{property_key}\s*\]\s*:)", - rf"{wrapped_receiver}{_JAVASCRIPT_MUTATION_TRIVIA}\.\s*" - rf"__define(?:Getter|Setter)__\s*\(\s*{property_key}\s*,", - ) - return any(re.search(pattern, content, re.MULTILINE) for pattern in patterns) - - -def _javascript_regexp_exec_mutation_possible(contents: tuple[str, ...]) -> bool: - """Return whether visible code may replace the method used by regexp literals. - - Prototype acquisition and mutation may occur in different components, so - the graph node evaluates these signals across the complete scanned skill. - The public ``analyze`` helper applies the same rule within a single source. - """ - if not contents: - return False - if not any( - any( - hint in content - for hint in ("RegExp", "getPrototypeOf", "__proto__", "constructor", "\\u", "\\x") - ) - for content in contents - ): - return False - if not any( - any( - hint in content - for hint in ( - "=", - "delete", - "defineProperty", - "defineProperties", - "assign", - "set", - "__define", - ) - ) - for content in contents - ): - return False - - scan_contents = tuple( - dict.fromkeys( - variant - for content in contents - for variant in _javascript_mutation_scan_variants(content) - ) - ) - property_aliases = _javascript_exec_property_aliases(scan_contents) - receiver = _javascript_regexp_prototype_receiver(scan_contents) - return any( - _javascript_mutates_regexp_exec(content, receiver, property_aliases) - for content in scan_contents - ) - - -class _OutputHandlingPatternAdapter: - """Bind whole-skill mutation context to the generic static runner contract.""" - - ANALYZER_ID = "static_patterns_output_handling" - - def __init__(self, regexp_exec_mutation_possible: bool) -> None: - self._regexp_exec_mutation_possible = regexp_exec_mutation_possible - - def analyze(self, *, content: str, file_path: str, file_type: str) -> list[AnalyzerFinding]: - """Analyze one component with whole-skill RegExp mutation context.""" - return analyze( - content, - file_path, - file_type, - regexp_exec_mutation_possible=self._regexp_exec_mutation_possible, - ) - - def _skip_javascript_whitespace_backward(content: str, index: int, floor: int) -> int: """Skip JavaScript whitespace before *index*, but deliberately not comments. @@ -649,6 +377,12 @@ def _is_javascript_regexp_literal_exec( and parentheses around the literal. It deliberately rejects comments and a parenthesized function argument such as ``makeRunner(/x/).exec(output)``. Contexts that require matching an earlier control header remain findings. + + This is a syntactic classification of the ordinary built-in method. Raw + cross-file source cannot establish whether, or when, another component + mutates JavaScript intrinsics; correlating those strings here would turn + uncertainty into an unverified HIGH finding at this call site. Prototype + mutation belongs in a separate parser-backed rule with data-flow context. """ if not _is_javascript_source(file_path, file_type): return False @@ -786,21 +520,10 @@ def _analyze_python_subprocess_calls( return findings -def analyze( - content: str, - file_path: str, - file_type: str, - *, - regexp_exec_mutation_possible: bool | None = None, -) -> list[AnalyzerFinding]: +def analyze(content: str, file_path: str, file_type: str) -> list[AnalyzerFinding]: """Analyze content for output handling patterns (OH1–OH3).""" findings: list[AnalyzerFinding] = [] - if regexp_exec_mutation_possible is None: - regexp_exec_mutation_possible = _is_javascript_source( - file_path, file_type - ) and _javascript_regexp_exec_mutation_possible((content,)) - def loc(ln: int) -> Location: return Location(file=file_path, start_line=ln) @@ -811,10 +534,8 @@ def ctx(start: int) -> str: for pattern, confidence in OH1_PATTERNS: for match in re.finditer(pattern, content, re.IGNORECASE | re.MULTILINE): - if ( - pattern == _EXEC_OUTPUT_PATTERN - and not regexp_exec_mutation_possible - and _is_javascript_regexp_literal_exec(content, match, file_path, file_type) + if pattern == _EXEC_OUTPUT_PATTERN and _is_javascript_regexp_literal_exec( + content, match, file_path, file_type ): continue line_num = get_line_number(content, match.start()) @@ -878,26 +599,6 @@ def ctx(start: int) -> str: def node(state: SkillspectorState) -> AnalyzerNodeResponse: """Run output_handling patterns and return findings.""" - components: list[str] = state.get("components") or [] - file_cache: dict[str, str] = state.get("file_cache") or {} - javascript_contents: list[str] = [] - has_uninspected_javascript = False - for path in components: - if not _is_javascript_source(path, ""): - continue - content = file_cache.get(path) - if ( - content is None - or len(content) > static_runner.MAX_FILE_CHARS - or "\x00" in content[:512] - ): - has_uninspected_javascript = True - else: - javascript_contents.append(content) - adapter = _OutputHandlingPatternAdapter( - has_uninspected_javascript - or _javascript_regexp_exec_mutation_possible(tuple(javascript_contents)) - ) - response = static_runner.run_static_patterns_with_ledger(state, [adapter]) + response = static_runner.run_static_patterns_with_ledger(state, [sys.modules[__name__]]) logger.info("%s: %d findings", ANALYZER_ID, len(response["findings"])) return response diff --git a/tests/unit/test_patterns_new.py b/tests/unit/test_patterns_new.py index 42d2ffe92..d3b9eb53f 100644 --- a/tests/unit/test_patterns_new.py +++ b/tests/unit/test_patterns_new.py @@ -390,176 +390,6 @@ def test_comment_separated_regexp_exec_fails_closed(self, content: str) -> None: assert any(f.rule_id == "OH1" for f in findings) - @pytest.mark.parametrize( - "mutation", - [ - pytest.param("RegExp.prototype.exec = eval;", id="direct_assignment"), - pytest.param('RegExp.prototype["exec"] = eval;', id="bracket_assignment"), - pytest.param("RegExp.prototype[`exec`] = eval;", id="template_key_assignment"), - pytest.param( - 'Object.defineProperty(RegExp.prototype, "exec", { value: eval });', - id="define_property", - ), - pytest.param( - 'Reflect.defineProperty(RegExp.prototype, "exec", { value: eval });', - id="reflect_define_property", - ), - pytest.param( - "Object.defineProperties(RegExp.prototype, { exec: { value: eval } });", - id="define_properties", - ), - pytest.param( - 'Reflect.set(RegExp.prototype, "exec", eval);', - id="reflect_set", - ), - pytest.param( - "Object.assign(RegExp.prototype, { exec: eval });", - id="object_assign", - ), - pytest.param( - "const regexPrototype = RegExp.prototype; regexPrototype.exec = eval;", - id="prototype_alias", - ), - pytest.param( - "const π = RegExp.prototype; π.exec = eval;", - id="unicode_prototype_alias", - ), - pytest.param( - "const NativeRegExp = RegExp; NativeRegExp.prototype.exec = eval;", - id="constructor_alias", - ), - pytest.param( - "const regexPrototype = Object.getPrototypeOf(/x/); regexPrototype.exec = eval;", - id="get_prototype_alias", - ), - pytest.param( - "const regexPrototype = /x/.__proto__; regexPrototype.exec = eval;", - id="legacy_proto_alias", - ), - pytest.param( - "const regex = /x/; regex.__proto__.exec = eval;", - id="regexp_instance_alias", - ), - pytest.param( - "const regexPrototype = Object.getPrototypeOf(new RegExp()); " - "regexPrototype.exec = eval;", - id="constructor_instance_alias", - ), - pytest.param( - "Object.getPrototypeOf(/x/).exec = eval;", - id="direct_get_prototype", - ), - pytest.param( - "(/x/).constructor.prototype.exec = eval;", - id="literal_constructor_prototype", - ), - pytest.param( - "Object.prototype.exec = eval; delete RegExp.prototype.exec;", - id="delete_to_inherited_exec", - ), - pytest.param( - "RegExp /* gap */ . /* gap */ prototype . /* gap */ exec /* gap */ = eval;", - id="comment_separated_assignment", - ), - pytest.param( - r"RegExp.prot\u006ftype.ex\u0065c = eval;", - id="identifier_unicode_escapes", - ), - pytest.param( - r'RegExp["prot\u006ftype"]["ex\u0065c"] = eval;', - id="string_unicode_escapes", - ), - pytest.param( - 'const property = "exec"; RegExp.prototype[property] = eval;', - id="computed_property_alias", - ), - pytest.param( - 'const property = "ex" + "ec"; RegExp.prototype[property] = eval;', - id="concatenated_property_alias", - ), - pytest.param( - 'Object.defineProperty(RegExp.prototype, "ex" + "ec", { value: eval });', - id="computed_define_property", - ), - pytest.param( - 'RegExp.prototype.__defineGetter__("exec", () => eval);', - id="legacy_define_getter", - ), - ], - ) - def test_regexp_literal_exec_fails_closed_when_prototype_may_be_mutated( - self, mutation: str - ) -> None: - content = f'{mutation}\nconst output = "globalThis.compromised = true";\n/x/.exec(output);' - - findings = oh_mod.analyze(content, "attack.js", "javascript") - - assert any(f.rule_id == "OH1" for f in findings) - - def test_regexp_exec_prototype_mutation_fails_closed_across_files(self) -> None: - response = oh_mod.node( - { - "components": ["prototype.js", "mutation.js", "parser.js"], - "file_cache": { - "prototype.js": "export const regexPrototype = RegExp.prototype;", - "mutation.js": ( - 'import { regexPrototype } from "./prototype.js";\n' - "regexPrototype.exec = eval;" - ), - "parser.js": ( - 'const output = "globalThis.compromised = true";\n/x/.exec(output);' - ), - }, - } - ) - - assert any( - finding.rule_id == "OH1" and finding.file == "parser.js" - for finding in response["findings"] - ) - - def test_unrelated_exec_assignment_does_not_disable_regexp_literal_exemption( - self, - ) -> None: - response = oh_mod.node( - { - "components": ["runner.js", "parser.js"], - "file_cache": { - "runner.js": "runner.exec = handler;", - "parser.js": "const match = /x/.exec(output);", - }, - } - ) - - assert not any(finding.rule_id == "OH1" for finding in response["findings"]) - - def test_reading_regexp_prototype_does_not_disable_literal_exemption(self) -> None: - response = oh_mod.node( - { - "components": ["introspection.js", "parser.js"], - "file_cache": { - "introspection.js": "const descriptor = RegExp.prototype.exec;", - "parser.js": "const match = /x/.exec(output);", - }, - } - ) - - assert not any(finding.rule_id == "OH1" for finding in response["findings"]) - - def test_unrelated_prototype_read_and_exec_write_do_not_combine(self) -> None: - response = oh_mod.node( - { - "components": ["introspection.js", "runner.js", "parser.js"], - "file_cache": { - "introspection.js": "const descriptor = RegExp.prototype.exec;", - "runner.js": "runner.exec = handler;", - "parser.js": "const match = /x/.exec(output);", - }, - } - ) - - assert not any(finding.rule_id == "OH1" for finding in response["findings"]) - @pytest.mark.parametrize( "uninspected_content", [ @@ -571,7 +401,7 @@ def test_unrelated_prototype_read_and_exec_write_do_not_combine(self) -> None: ), ], ) - def test_uninspected_javascript_sibling_fails_closed( + def test_uninspected_sibling_does_not_invent_oh1_at_regexp_call( self, uninspected_content: str | None ) -> None: file_cache = {"parser.js": "const match = /x/.exec(output);"} @@ -585,11 +415,28 @@ def test_uninspected_javascript_sibling_fails_closed( } ) - assert any( + assert not any( finding.rule_id == "OH1" and finding.file == "parser.js" for finding in response["findings"] ) + @pytest.mark.parametrize( + "context", + [ + pytest.param( + 'const note = "RegExp.prototype.exec = eval;";', + id="string_literal", + ), + pytest.param("// RegExp.prototype.exec = eval;", id="line_comment"), + ], + ) + def test_mutation_shaped_text_does_not_invent_oh1_at_regexp_call(self, context: str) -> None: + content = f"{context}\nconst match = /x/.exec(output);" + + findings = oh_mod.analyze(content, "parser.js", "javascript") + + assert not any(f.rule_id == "OH1" for f in findings) + def test_python_exec_remains_output_injection(self) -> None: findings = oh_mod.analyze("exec(output)", "runner.py", "python") From 1c2f29b9fdcfe3ca0878c12fab1c63907f3550ba Mon Sep 17 00:00:00 2001 From: Christopher Kevin Date: Tue, 4 Aug 2026 04:22:10 -0700 Subject: [PATCH 4/6] test(output-handling): cover regexp class syntax Signed-off-by: Christopher Kevin --- tests/unit/test_patterns_new.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/unit/test_patterns_new.py b/tests/unit/test_patterns_new.py index d3b9eb53f..0df85dc9e 100644 --- a/tests/unit/test_patterns_new.py +++ b/tests/unit/test_patterns_new.py @@ -263,6 +263,14 @@ def test_reported_regexp_literal_exec_is_not_output_injection(self) -> None: pytest.param("const match = ((/error/i)).exec(output);", id="parenthesized"), pytest.param("const match = /error/i\n .exec(output);", id="line_broken"), pytest.param(r"const match = /[\/]/u.exec(output);", id="character_class_slash"), + pytest.param( + r"const match = /[/]/u.exec(output);", + id="character_class_unescaped_slash", + ), + pytest.param( + r"const match = /[[A-z]--_]/v.exec(output);", + id="unicode_sets_nested_class", + ), pytest.param("const match = (/error/i)?.exec(output);", id="optional_chain"), pytest.param( 'const url = "https://example.test"; const match = /error/i.exec(output);', From a11bbcb0cf58a459e354e7ad8784aecf088559e0 Mon Sep 17 00:00:00 2001 From: Christopher Kevin Date: Tue, 4 Aug 2026 12:59:04 -0700 Subject: [PATCH 5/6] fix(output-handling): fail closed across line comments Signed-off-by: Christopher Kevin --- .../static_patterns_output_handling.py | 73 +++++++++++++++++++ tests/unit/test_patterns_new.py | 64 ++++++++++++++++ 2 files changed, 137 insertions(+) diff --git a/src/skillspector/nodes/analyzers/static_patterns_output_handling.py b/src/skillspector/nodes/analyzers/static_patterns_output_handling.py index 09535451c..6ee0e4d7e 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_output_handling.py +++ b/src/skillspector/nodes/analyzers/static_patterns_output_handling.py @@ -217,6 +217,77 @@ def _skip_javascript_whitespace_backward(content: str, index: int, floor: int) - return index +def _javascript_whitespace_crosses_possible_line_comment( + content: str, whitespace_start: int, whitespace_end: int, floor: int +) -> bool: + """Return whether a backward whitespace walk may have entered ``//`` text. + + A line comment ends at a JavaScript line terminator. After walking backward + across that terminator, an accepted expression-prefix character or keyword + at the end of the comment must not validate the following slash as a regexp + literal. Ordinary quoted strings are tracked so a URL on the preceding line + does not look like a comment. Definite comment openers fail closed. A prior + unquoted slash only becomes ambiguous if later quoting prevents this small + scanner from proving that a subsequent ``//`` is outside a regexp. Lines + that inherit a multiline string, template, or block-comment state and + truncated lines also fail closed. + """ + whitespace = content[whitespace_start:whitespace_end] + if not any(terminator in whitespace for terminator in _JAVASCRIPT_LINE_TERMINATORS): + return False + + last_line_break = max( + content.rfind(terminator, floor, whitespace_start) + for terminator in _JAVASCRIPT_LINE_TERMINATORS + ) + if last_line_break >= floor: + line_start = last_line_break + 1 + elif floor == 0 or content[floor - 1] in _JAVASCRIPT_LINE_TERMINATORS: + line_start = floor + else: + return True + + line_prefix = content[line_start:whitespace_start] + if "`" in line_prefix or "*/" in line_prefix: + return True + if last_line_break >= floor: + terminator_start = last_line_break + while ( + terminator_start > floor + and content[terminator_start - 1] in _JAVASCRIPT_LINE_TERMINATORS + ): + terminator_start -= 1 + if _is_javascript_character_escaped(content, terminator_start, floor): + return True + + quote: str | None = None + escaped = False + saw_unquoted_slash = False + cursor = line_start + while cursor < whitespace_start: + character = content[cursor] + if quote is not None: + if escaped: + escaped = False + elif character == "\\": + escaped = True + elif character == quote: + quote = None + elif character in {'"', "'"}: + if saw_unquoted_slash: + return True + quote = character + elif character == "`": + return True + elif character == "/": + if cursor + 1 < whitespace_start and content[cursor + 1] in {"/", "*"}: + return True + saw_unquoted_slash = True + cursor += 1 + + return quote is not None + + def _is_javascript_character_escaped(content: str, index: int, floor: int) -> bool: """Return whether the character at *index* has an odd backslash prefix.""" backslashes = 0 @@ -327,6 +398,8 @@ def _find_javascript_regexp_opening_slash( def _javascript_expression_can_start_at(content: str, index: int, floor: int) -> bool: """Conservatively validate that a JavaScript expression may start at *index*.""" cursor = _skip_javascript_whitespace_backward(content, index, floor) + if _javascript_whitespace_crosses_possible_line_comment(content, cursor, index, floor): + return False if cursor == floor: return floor == 0 diff --git a/tests/unit/test_patterns_new.py b/tests/unit/test_patterns_new.py index 0df85dc9e..c2d5f7c0e 100644 --- a/tests/unit/test_patterns_new.py +++ b/tests/unit/test_patterns_new.py @@ -262,6 +262,10 @@ def test_reported_regexp_literal_exec_is_not_output_injection(self) -> None: ), pytest.param("const match = ((/error/i)).exec(output);", id="parenthesized"), pytest.param("const match = /error/i\n .exec(output);", id="line_broken"), + pytest.param( + "const match =\n /error/i.exec(output);", + id="literal_after_assignment_line_break", + ), pytest.param(r"const match = /[\/]/u.exec(output);", id="character_class_slash"), pytest.param( r"const match = /[/]/u.exec(output);", @@ -280,6 +284,22 @@ def test_reported_regexp_literal_exec_is_not_output_injection(self) -> None: 'const url = "https://example.test"; const match = /error/i\n .exec(output);', id="url_string_before_line_break", ), + pytest.param( + 'const url = "https://example.test";\n/error/i.exec(output);', + id="url_string_statement_before_literal_line_break", + ), + pytest.param( + "const prior = 8 / 2;\n/error/i.exec(output);", + id="division_statement_before_literal_line_break", + ), + pytest.param( + "const match = 8 / 2 +\n/error/i.exec(output);", + id="division_before_multiline_literal_operand", + ), + pytest.param( + "/prefix/.test(output);\n/error/i.exec(output);", + id="regexp_statement_before_literal_line_break", + ), pytest.param("return (/error/i).exec(output);", id="grouped_return"), pytest.param("throw (/error/i).exec(output);", id="grouped_throw"), pytest.param("typeof (/error/i).exec(output);", id="grouped_unary_keyword"), @@ -380,6 +400,50 @@ def test_dangerous_exec_sinks_remain_output_injection(self, content: str) -> Non assert any(f.rule_id == "OH1" for f in findings) + @pytest.mark.parametrize( + "content", + [ + pytest.param("left // TODO:\n/right/g.exec(output)", id="punctuation_lf"), + pytest.param("left // return\n/right/g.exec(output)", id="keyword_lf"), + pytest.param("left // TODO:\r/right/g.exec(output)", id="punctuation_cr"), + pytest.param("left // TODO:\r\n/right/g.exec(output)", id="punctuation_crlf"), + pytest.param("left // TODO:\u2028/right/g.exec(output)", id="punctuation_ls"), + pytest.param("left // TODO:\u2029/right/g.exec(output)", id="punctuation_ps"), + pytest.param( + "left / /'/.source // ':\n/right/g.exec(output)", + id="comment_after_regexp_quote", + ), + pytest.param( + "/* open\n' */ left // ':\n/right/g.exec(output)", + id="comment_after_multiline_block_comment_quote", + ), + pytest.param( + "const value = 'continued\\\n'; left // ':\n/right/g.exec(output)", + id="comment_after_continued_string_quote", + ), + pytest.param( + "const value = 'continued\\\r\n'; left // ':\r\n/right/g.exec(output)", + id="comment_after_crlf_continued_string_quote", + ), + pytest.param( + "const value = `continued\n'`; left // ':\n/right/g.exec(output)", + id="comment_after_multiline_template_quote", + ), + ], + ) + def test_line_comment_before_regexp_shaped_division_fails_closed(self, content: str) -> None: + findings = oh_mod.analyze(content, "runner.ts", "typescript") + + assert any(f.rule_id == "OH1" for f in findings) + + def test_line_comment_detection_fails_closed_at_lookback_boundary(self) -> None: + prefix = "x" * (oh_mod._JAVASCRIPT_REGEXP_LOOKBACK_CHARS + 32) + content = f"{prefix} // return\n/right/g.exec(output)" + + findings = oh_mod.analyze(content, "runner.ts", "typescript") + + assert any(f.rule_id == "OH1" for f in findings) + @pytest.mark.parametrize( "content", [ From a0768f0b46c8659c28a86c314131bec6c8d27d5f Mon Sep 17 00:00:00 2001 From: Christopher Kevin Date: Tue, 4 Aug 2026 14:03:12 -0700 Subject: [PATCH 6/6] fix(output-handling): handle legacy HTML comments Signed-off-by: Christopher Kevin --- .../static_patterns_output_handling.py | 11 +++-- tests/unit/test_patterns_new.py | 44 +++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/skillspector/nodes/analyzers/static_patterns_output_handling.py b/src/skillspector/nodes/analyzers/static_patterns_output_handling.py index 6ee0e4d7e..934ddc8e2 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_output_handling.py +++ b/src/skillspector/nodes/analyzers/static_patterns_output_handling.py @@ -220,13 +220,14 @@ def _skip_javascript_whitespace_backward(content: str, index: int, floor: int) - def _javascript_whitespace_crosses_possible_line_comment( content: str, whitespace_start: int, whitespace_end: int, floor: int ) -> bool: - """Return whether a backward whitespace walk may have entered ``//`` text. + """Return whether a backward whitespace walk may have entered a line comment. A line comment ends at a JavaScript line terminator. After walking backward across that terminator, an accepted expression-prefix character or keyword at the end of the comment must not validate the following slash as a regexp - literal. Ordinary quoted strings are tracked so a URL on the preceding line - does not look like a comment. Definite comment openers fail closed. A prior + literal. This includes Annex B's legacy ```` closer. Ordinary quoted strings are tracked so comment lookalikes on + the preceding line do not fail closed. Definite comment openers do. A prior unquoted slash only becomes ambiguous if later quoting prevents this small scanner from proving that a subsequent ``//`` is outside a regexp. Lines that inherit a multiline string, template, or block-comment state and @@ -250,6 +251,8 @@ def _javascript_whitespace_crosses_possible_line_comment( line_prefix = content[line_start:whitespace_start] if "`" in line_prefix or "*/" in line_prefix: return True + if line_prefix.lstrip().startswith("-->"): + return True if last_line_break >= floor: terminator_start = last_line_break while ( @@ -279,6 +282,8 @@ def _javascript_whitespace_crosses_possible_line_comment( quote = character elif character == "`": return True + elif content.startswith(" return";\n/error/i.exec(output);', + id="quoted_html_close_comment_lookalike", + ), + pytest.param( + "const compared = left-- > right;\n/error/i.exec(output);", + id="postfix_decrement_comparison_before_literal", + ), pytest.param("return (/error/i).exec(output);", id="grouped_return"), pytest.param("throw (/error/i).exec(output);", id="grouped_throw"), pytest.param("typeof (/error/i).exec(output);", id="grouped_unary_keyword"), @@ -436,6 +448,38 @@ def test_line_comment_before_regexp_shaped_division_fails_closed(self, content: assert any(f.rule_id == "OH1" for f in findings) + @pytest.mark.parametrize( + "terminator", + [ + pytest.param("\n", id="lf"), + pytest.param("\r", id="cr"), + pytest.param("\r\n", id="crlf"), + pytest.param("\u2028", id="ls"), + pytest.param("\u2029", id="ps"), + ], + ) + @pytest.mark.parametrize( + "content_template", + [ + pytest.param( + "left return{terminator}/right/g.exec(output)", + id="html_close_comment", + ), + ], + ) + def test_legacy_html_comment_before_regexp_shaped_division_fails_closed( + self, content_template: str, terminator: str + ) -> None: + content = content_template.format(terminator=terminator) + + findings = oh_mod.analyze(content, "runner.js", "javascript") + + assert any(f.rule_id == "OH1" for f in findings) + def test_line_comment_detection_fails_closed_at_lookback_boundary(self) -> None: prefix = "x" * (oh_mod._JAVASCRIPT_REGEXP_LOOKBACK_CHARS + 32) content = f"{prefix} // return\n/right/g.exec(output)"