fix(plugin-guard): two intake false positives — regex literal <script, allowlist "printenv"
Desktop lint: /<script[\s\S]*?<\/script>/gi in a feed sanitiser scored as
'script injection' and failed pinned-source-validate for rss-reader. Mask
JS regex literals for the markup-shaped rule only; <script in a string
literal (an innerHTML payload) and createElement('script') still fail.
Install scanner: "printenv" as a whole-string entry of a read-only
allowlist (frozenset({..., "printenv"})) fired dump_all_env high →
caution on hermes-jev. Extend the literal-token demotion: a token that is
the ENTIRE quoted literal on a line that executes nothing steps down like
an alternation member; "sudo" inside subprocess.run([...]) and
os.system("printenv") keep high.
A/B vs origin/main: attack probes identical (23 rows), in-tree sweep 319
entries 0 worse/0 changed; both new tests red on base. Bumps
PLUGIN_SCANNER_VERSION to v6 so cached caution verdicts refresh.
Signed-off-by: teknium1 <teknium1@users.noreply.github.com>
This commit is contained in:
@@ -31,14 +31,28 @@ _FORBIDDEN: Tuple[Tuple[str, "re.Pattern[str]"], ...] = (
|
||||
|
||||
_COMMENT = re.compile(r"/\*.*?\*/|(?<![:\w])//[^\n]*", re.S)
|
||||
|
||||
# A JS regex literal (``/<script[\s\S]*?<\/script>/gi``) matches markup, it cannot inject any: a
|
||||
# feed sanitiser that STRIPS script tags is the opposite of the move the rule refuses. Regex
|
||||
# literals are masked for the markup-shaped rules only; a ``<script`` inside a string literal is
|
||||
# still the payload of an ``innerHTML`` write and keeps firing. The lookbehind keeps division
|
||||
# (``a / b / c``) from reading as a literal.
|
||||
_REGEX_LITERAL = re.compile(r"(?<![\w)\]])/(?:[^/\\\n\[]|\\.|\[(?:[^\]\\\n]|\\.)*\])+/[a-z]*")
|
||||
_MARKUP_RULES = frozenset({"script injection"})
|
||||
|
||||
|
||||
def _mask_regex_literals(source: str) -> str:
|
||||
return _REGEX_LITERAL.sub(lambda m: " " * len(m.group(0)), source)
|
||||
|
||||
|
||||
def desktop_surface_findings(source: str) -> List[Tuple[str, int]]:
|
||||
"""Return ``[(rule, line)]`` for every forbidden construct in a plugin.js source."""
|
||||
stripped = _COMMENT.sub(lambda m: "\n" * m.group(0).count("\n"), source)
|
||||
no_regex = _mask_regex_literals(stripped)
|
||||
findings: List[Tuple[str, int]] = []
|
||||
for rule, pattern in _FORBIDDEN:
|
||||
for match in pattern.finditer(stripped):
|
||||
findings.append((rule, stripped.count("\n", 0, match.start()) + 1))
|
||||
haystack = no_regex if rule in _MARKUP_RULES else stripped
|
||||
for match in pattern.finditer(haystack):
|
||||
findings.append((rule, haystack.count("\n", 0, match.start()) + 1))
|
||||
return sorted(findings, key=lambda f: f[1])
|
||||
|
||||
|
||||
|
||||
@@ -222,6 +222,20 @@ class TestDesktopSurface:
|
||||
report = validate_plugin_dir(d)
|
||||
assert ("desktop surface", True, "stays inside the plugin SDK surface") in report.checks
|
||||
|
||||
def test_script_regex_literal_is_not_injection_but_string_is(self, tmp_path):
|
||||
d = self._desktop_plugin(tmp_path, (
|
||||
"const clean = html.replace(/<script[\\s\\S]*?<\\/script>/gi, '').replace(/<style[\\s\\S]*?<\\/style>/gi, '')\n"
|
||||
"const ratio = total / count / 2\n"
|
||||
"el.innerHTML = '<script src=\"https://evil.example/x.js\"></script>'\n"
|
||||
"const tag = document.createElement('script')\n"
|
||||
))
|
||||
report = validate_plugin_dir(d)
|
||||
failed = {name: detail for name, ok, detail in report.checks if not ok}
|
||||
assert "desktop surface" in failed
|
||||
assert ":1)" not in failed["desktop surface"]
|
||||
assert "script injection (desktop/plugin.js:3)" in failed["desktop surface"]
|
||||
assert "script injection (desktop/plugin.js:4)" in failed["desktop surface"]
|
||||
|
||||
def test_prototype_patch_and_chunk_import_fail(self, tmp_path):
|
||||
d = self._desktop_plugin(tmp_path, (
|
||||
"const raw = Storage.prototype.setItem\n"
|
||||
|
||||
@@ -534,6 +534,22 @@ class TestInertContextDemotions:
|
||||
assert sev[("redact.py", "dump_all_env")] == "medium"
|
||||
assert sev[("priv.py", "sudo_usage")] == "high"
|
||||
|
||||
def test_whole_literal_list_entry_vs_executed_literal(self, tmp_path):
|
||||
files = dict(BASE_FILES)
|
||||
files["gate.py"] = (
|
||||
"_READ_ONLY = frozenset({\n"
|
||||
' "id", "uname", "uptime", "free", "ps", "printenv",\n'
|
||||
"})\n"
|
||||
"DENY = [\"sudo\", \"rm\"]\n"
|
||||
)
|
||||
files["run.py"] = 'subprocess.run(["sudo", "-n", "true"])\nos.system("printenv")\n'
|
||||
result = scan_plugin(_mk_plugin(tmp_path, files), source="owner/repo")
|
||||
sev = {(f.file, f.pattern_id): f.severity for f in result.findings}
|
||||
assert sev[("gate.py", "dump_all_env")] == "medium" # allowlist entry: a note
|
||||
assert sev[("gate.py", "sudo_usage")] == "medium" # denylist entry: a note
|
||||
assert sev[("run.py", "sudo_usage")] == "high" # argv passed to run(): executes
|
||||
assert sev[("run.py", "dump_all_env")] == "high" # os.system("printenv"): executes
|
||||
|
||||
def test_base64_decode_to_text_filter_vs_interpreter(self, tmp_path):
|
||||
files = dict(BASE_FILES)
|
||||
files["scripts/open-pr.sh"] = "gh api repos/x/contents/y --jq .content | base64 -d | grep '^sha:'\n"
|
||||
|
||||
@@ -22,7 +22,7 @@ from tools.skills_guard import (
|
||||
Finding, ScanResult, SUSPICIOUS_BINARY_EXTENSIONS, _determine_verdict, format_scan_report,
|
||||
scan_file)
|
||||
|
||||
PLUGIN_SCANNER_VERSION = "plugin-guard-v5"
|
||||
PLUGIN_SCANNER_VERSION = "plugin-guard-v6"
|
||||
|
||||
# Never scanned: VCS internals, caches, vendored envs.
|
||||
EXCLUDED_DIRS = {
|
||||
|
||||
@@ -157,11 +157,13 @@ def is_base64_media(line: str) -> bool:
|
||||
|
||||
# ── (5)/(6) alternation tokens inside string or regex literals in code ──────────────────────
|
||||
# ``sudo`` in ``/clarify|approval|sudo|secret/.test(value)`` classifies an event name; ``env|``
|
||||
# in ``re.compile(r"(?:api[_-]?key|…|env|headers)")`` is a redaction regex. The shape that is
|
||||
# inert is narrow: the word sits inside a quoted string or regex literal AND is an alternation
|
||||
# member (``|sudo|``, ``(sudo|``, ``|env|``). A command string such as ``"sudo apt install x"``
|
||||
# or ``"env | grep KEY"`` inside a ``subprocess.run(...)`` literal is how an attack is written
|
||||
# and never qualifies. Only word-shaped patterns are eligible.
|
||||
# in ``re.compile(r"(?:api[_-]?key|…|env|headers)")`` is a redaction regex; ``"printenv",`` in
|
||||
# ``_READ_ONLY_COMMANDS = frozenset({"pwd", "ls", …, "printenv"})`` is a denylist/allowlist entry.
|
||||
# The shape that is inert is narrow: the word sits inside a quoted string or regex literal AND is
|
||||
# either an alternation member (``|sudo|``, ``(sudo|``, ``|env|``) or the ENTIRE literal
|
||||
# (``"printenv"``, ``'sudo'``) on a line that executes nothing. A command string such as
|
||||
# ``"sudo apt install x"`` or ``"env | grep KEY"`` inside a ``subprocess.run(...)`` literal is how
|
||||
# an attack is written and never qualifies. Only word-shaped patterns are eligible.
|
||||
LITERAL_INERT_PATTERN_IDS = {"sudo_usage", "dump_all_env"}
|
||||
_LITERAL_SPANS = re.compile(
|
||||
r"""(?P<s>[rRbBuUfF]{0,2}"(?:[^"\\\n]|\\.)*"|[rRbBuUfF]{0,2}'(?:[^'\\\n]|\\.)*'|`(?:[^`\\\n]|\\.)*`)"""
|
||||
@@ -176,18 +178,30 @@ def _is_alternation_member(line: str, start: int, end: int) -> bool:
|
||||
return before in "|(" or after in "|)"
|
||||
|
||||
|
||||
def _is_whole_literal(line: str, start: int, end: int, span: tuple[int, int]) -> bool:
|
||||
"""The token is the entire quoted content of the literal it sits in (``"printenv"``)."""
|
||||
a, b = span
|
||||
return start == a + 1 and end == b - 1 and line[a] in "\"'`" and not _EXEC_ON_LINE.search(line)
|
||||
|
||||
|
||||
def is_regex_alternation_token(finding: Finding, line: str) -> bool:
|
||||
"""Every occurrence of the finding's token sits inside a literal as an alternation member."""
|
||||
"""Every occurrence of the finding's token sits inside a literal as an alternation member
|
||||
or as the whole literal (a list entry) on a line that executes nothing."""
|
||||
token = _PATTERN_TOKEN.get(finding.pattern_id)
|
||||
if token is None:
|
||||
return False
|
||||
spans = [m.span() for m in _LITERAL_SPANS.finditer(line)]
|
||||
hits = list(token.finditer(line))
|
||||
return bool(hits) and all(
|
||||
any(a <= h.start() and h.end() <= b for a, b in spans)
|
||||
and " " not in h.group(0) and _is_alternation_member(line, h.start(), h.end())
|
||||
for h in hits
|
||||
)
|
||||
|
||||
def inert(h: "re.Match[str]") -> bool:
|
||||
if " " in h.group(0):
|
||||
return False
|
||||
span = next(((a, b) for a, b in spans if a <= h.start() and h.end() <= b), None)
|
||||
if span is None:
|
||||
return False
|
||||
return _is_alternation_member(line, h.start(), h.end()) or _is_whole_literal(line, h.start(), h.end(), span)
|
||||
|
||||
return bool(hits) and all(inert(h) for h in hits)
|
||||
|
||||
|
||||
# ── (6) base64 decode piped to a non-interpreter ────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user