fix(behavioral-ast): detect reflective exec via getattr() literal (AST9)

README.md

@@ -18,7 +18,7 @@ SkillSpector helps you answer: **"Is this skill safe to install?"**
 ## Features
 
 - **Multi-format input**: Scan Git repos, URLs, zip files, directories, or single files
-- **64 vulnerability patterns** across 16 categories: prompt injection, data exfiltration, privilege escalation, supply chain, excessive agency, output handling, system prompt leakage, memory poisoning, tool misuse, rogue agent, trigger abuse, dangerous code (AST), taint tracking, YARA signatures, MCP least privilege, and MCP tool poisoning
+- **65 vulnerability patterns** across 16 categories: prompt injection, data exfiltration, privilege escalation, supply chain, excessive agency, output handling, system prompt leakage, memory poisoning, tool misuse, rogue agent, trigger abuse, dangerous code (AST), taint tracking, YARA signatures, MCP least privilege, and MCP tool poisoning
 - **Two-stage analysis**: Fast static analysis + optional LLM semantic evaluation
 - **Live vulnerability lookups**: SC4 queries [OSV.dev](https://osv.dev) for real-time CVE data with automatic offline fallback
 - **Multiple output formats**: Terminal, JSON, Markdown, and SARIF reports
@@ -200,7 +200,7 @@ skillspector scan ./my-skill/ --no-llm
 
 ## Vulnerability Patterns
 
-SkillSpector detects **64 vulnerability patterns** across 16 categories:
+SkillSpector detects **65 vulnerability patterns** across 16 categories:
 
 ### Prompt Injection (5 patterns)
 
@@ -296,7 +296,7 @@ SkillSpector detects **64 vulnerability patterns** across 16 categories:
 | TR2 | Shadow Command Trigger | HIGH | Triggers that shadow built-in commands or other skills |
 | TR3 | Keyword Baiting Trigger | MEDIUM | Generic triggers designed to maximize activation |
 
-### Behavioral AST (8 patterns)
+### Behavioral AST (9 patterns)
 
 | ID | Pattern | Severity | Description |
 |----|---------|----------|-------------|
@@ -308,6 +308,7 @@ SkillSpector detects **64 vulnerability patterns** across 16 categories:
 | AST6 | compile() Call | MEDIUM | Code object creation from strings |
 | AST7 | Dynamic getattr() | MEDIUM | Arbitrary attribute access with non-literal names |
 | AST8 | Dangerous Execution Chain | CRITICAL | exec/eval combined with dynamic source (network, encoded data) |
+| AST9 | Reflective getattr() Sink | HIGH | Reflective exec via `getattr(os,'system')` / `getattr(builtins,'exec')` that evades AST1/AST5 |
 
 ### Taint Tracking (5 patterns)
 

src/skillspector/nodes/analyzers/behavioral_ast.py

@@ -36,6 +36,16 @@
 
 _DANGEROUS_BUILTINS = frozenset({"exec", "eval", "compile", "__import__"})
 
+# Names that turn ``getattr(obj, "<name>")`` into a reflective handle on a code- or
+# command-execution sink. ``getattr(os, "system")(cmd)`` and
+# ``getattr(builtins, "exec")(src)`` are functionally identical to ``os.system(cmd)``
+# / ``exec(src)`` but evade AST1/AST5: the inner ``getattr`` has a *constant* second
+# argument (so AST7 is intentionally skipped), and the outer call's ``func`` is an
+# ``ast.Call`` whose name does not resolve, so AST1/AST5 never fire. The set is kept
+# deliberately small — only names with essentially no legitimate ``getattr`` use — so
+# benign reflection such as ``getattr(obj, "name")`` stays unflagged.
+_DANGEROUS_GETATTR_NAMES = frozenset({"exec", "eval", "system", "popen", "__import__"})
+
 _SUBPROCESS_CALLS = frozenset(
     {
         "call",
@@ -82,6 +92,7 @@
     "AST6": "compile() call detected",
     "AST7": "Dynamic attribute access via getattr()",
     "AST8": "Dangerous execution chain",
+    "AST9": "Reflective dangerous call via getattr() with a literal sink name",
 }
 
 _RULE_SEVERITIES: dict[str, Severity] = {
@@ -93,6 +104,7 @@
     "AST6": Severity.MEDIUM,
     "AST7": Severity.LOW,
     "AST8": Severity.CRITICAL,
+    "AST9": Severity.HIGH,
 }
 
 _RULE_CONFIDENCES: dict[str, float] = {
@@ -104,6 +116,7 @@
     "AST6": 0.65,
     "AST7": 0.50,
     "AST8": 0.95,
+    "AST9": 0.85,
 }
 
 _TAG = "Dangerous Code Execution"
@@ -206,8 +219,14 @@ def _emit(
                 _emit("AST5", lineno, end_lineno)
 
         elif call_name == "getattr" and len(ast_node.args) >= 2:
-            if not isinstance(ast_node.args[1], ast.Constant):
+            second_arg = ast_node.args[1]
+            if not isinstance(second_arg, ast.Constant):
                 _emit("AST7", lineno, end_lineno)
+            elif (
+                isinstance(second_arg.value, str)
+                and second_arg.value in _DANGEROUS_GETATTR_NAMES
+            ):
+                _emit("AST9", lineno, end_lineno)
 
     return findings
 

src/skillspector/nodes/analyzers/pattern_defaults.py

@@ -105,6 +105,7 @@ class PatternCategory(StrEnum):
     "AST6": "compile() creates code objects from strings. When combined with exec()/eval(), it enables obfuscated code execution.",
     "AST7": "Dynamic getattr() with a non-literal attribute name can access arbitrary object attributes, potentially bypassing access controls.",
     "AST8": "A dangerous execution chain combines code execution (exec/eval) with a dynamic source (network, encoded data, dynamic import), creating a high-confidence attack vector.",
+    "AST9": "Reflective access to an execution sink via getattr() with a constant name (e.g. getattr(os, 'system'), getattr(builtins, 'exec')) is functionally identical to a direct exec/os.system call but evades name-based detection. This is a deliberate evasion technique rather than idiomatic code.",
     # YARA (B.1.12)
     "YR1": "YARA rule matched a known malware signature (reverse shell, backdoor, ransomware, C2 framework, or info stealer).",
     "YR2": "YARA rule matched a known webshell pattern (PHP, Python, JSP, or ASPX webshell).",
@@ -318,6 +319,7 @@ class PatternCategory(StrEnum):
     "AST6": "Avoid compile() with dynamic strings. If code generation is needed, use templates or AST manipulation with strict validation.",
     "AST7": "Replace dynamic getattr() with explicit attribute access or a dictionary lookup with an allowlist of permitted attributes.",
     "AST8": "Remove the execution chain entirely. Never pass network data, decoded bytes, or dynamically imported code to exec()/eval(). Use structured data formats instead.",
+    "AST9": "Call the function directly instead of reflectively (write exec(...) / os.system(...) explicitly), or remove it. If reflection is genuinely required, restrict it to an allowlist of safe attribute names that excludes execution sinks.",
     # Behavioral Taint Tracking (B.2.2)
     "TT1": "Add validation or sanitization between the data source and sink. Never pass raw source data directly to a sink without checking its content.",
     "TT2": "Validate tainted variables before passing them to sinks. Use allowlists, type checks, or sanitization functions on data from external sources.",

tests/nodes/analyzers/test_behavioral_ast.py

@@ -120,6 +120,44 @@ def test_getattr_with_literal_no_finding(self):
         assert not any(f.rule_id == "AST7" for f in findings)
 
 
+class TestReflectiveGetattrExec:
+    """getattr(obj, "<sink>")(...) is a reflective handle on an exec/os sink.
+
+    It evades AST1/AST5 (the inner getattr has a *constant* name so AST7 is skipped,
+    and the outer call's func is an ast.Call whose name does not resolve), so it must
+    be caught directly as AST9.
+    """
+
+    def test_getattr_os_system_produces_ast9(self):
+        findings = _run("import os\ngetattr(os, 'system')('id')")
+        ast9 = [f for f in findings if f.rule_id == "AST9"]
+        assert len(ast9) == 1
+        assert ast9[0].severity == "HIGH"
+
+    def test_getattr_builtins_exec_produces_ast9(self):
+        findings = _run("import builtins\ngetattr(builtins, 'exec')(payload)")
+        assert any(f.rule_id == "AST9" for f in findings)
+
+    def test_getattr_eval_double_quotes_produces_ast9(self):
+        findings = _run('import builtins\ngetattr(builtins, "eval")("2+2")')
+        assert any(f.rule_id == "AST9" for f in findings)
+
+    def test_getattr_os_popen_produces_ast9(self):
+        findings = _run("import os\nhandle = getattr(os, 'popen')('whoami')")
+        assert any(f.rule_id == "AST9" for f in findings)
+
+    def test_reflective_getattr_does_not_emit_ast7(self):
+        # A constant name must not also trip the non-literal AST7 rule.
+        findings = _run("import os\ngetattr(os, 'system')('id')")
+        assert not any(f.rule_id == "AST7" for f in findings)
+
+    def test_benign_constant_attr_no_ast9(self):
+        # Common, safe reflective access must stay unflagged (near-zero false positives).
+        for name in ("name", "timeout", "value", "data", "run", "compile"):
+            findings = _run(f"v = getattr(config, '{name}')")
+            assert not any(f.rule_id == "AST9" for f in findings), name
+
+
 class TestDangerousChains:
     def test_exec_compile_chain_produces_ast8(self):
         code = 'exec(compile("x = 1", "<string>", "exec"))'