diff --git a/CHANGELOG.md b/CHANGELOG.md index d6c0cde..a010e93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/), and this project adheres to [Semantic Versioning](https://semver.org/). +## [Unreleased] + +### Fixed +- Fixed command-injection false negative when calling `os.system` / `os.popen` / `subprocess.getoutput` / `subprocess.getstatusoutput` / `child_process.exec` / `execSync` / `exec` / `spawn` with a simple variable argument. + ## [0.19.0] — 2026-09-22 Migration: **none required.** The one new surface is an opt-in flag, and nothing existing changes diff --git a/src/websec_validator/extractors/surface.py b/src/websec_validator/extractors/surface.py index 9b17062..36950b6 100644 --- a/src/websec_validator/extractors/surface.py +++ b/src/websec_validator/extractors/surface.py @@ -51,8 +51,8 @@ r"\s*\(\s*[^)\n;]{0,160}?" + _REQ_SRC)), "command-injection": ("ssrf-probes", None, re.compile( r"(?:child_process\.exec|\bexecSync|\bexec|\bspawn|os\.system|os\.popen" - r"|subprocess\.(?:run|call|check_output|Popen|getoutput|getstatusoutput))\s*\([^)]*" - + _U + r"|shell\s*=\s*True")), + r"|subprocess\.(?:run|call|check_output|Popen|getoutput|getstatusoutput))\s*\([^)]*(?:" + + _U + r"|shell\s*=\s*True)|(?:os\.(?:system|popen)|subprocess\.(?:getoutput|getstatusoutput)|child_process\.exec|\bexecSync|\bexec|\bspawn)\s*\(\s*[a-zA-Z_]\w*\s*\)")), "sql-injection": ("bola-write-verbs", "sql", re.compile( r"(?:\.query|\.execute|\.raw|cursor\.execute|sequelize\.query|knex\.raw)\s*\([^)]*(?:\$\{|\+|%\s*[\(%]|\.format\s*\(|f['\"])")), "nosql-injection": ("bola-write-verbs", "nosql", re.compile( diff --git a/tests/test_pentest_regressions.py b/tests/test_pentest_regressions.py index c88fc85..5dc6efd 100644 --- a/tests/test_pentest_regressions.py +++ b/tests/test_pentest_regressions.py @@ -132,6 +132,23 @@ def test_identity_bound_vtl_passes(self): self.assertTrue(any(s["verdict"] == "authz-bound" for s in out["subscription_authz"])) +class CommandInjectionTests(unittest.TestCase): + def test_simple_variable_detected_but_not_literals(self): + out = SurfaceExtractor().extract(repo({ + "vuln.py": "def run(cmd): return os.popen(cmd)\n", + "vuln2.py": "subprocess.getoutput(cmd)\n", + "vuln_js.ts": "exec(bar)\n", + "safe.py": "os.system('ls')\n", + "safe2.py": "subprocess.run(['ls'])\n"}), {"stack": {"datastores": []}}) + self.assertIn("command-injection", out["sinks"]) + files = out["sinks"]["command-injection"]["files"] + self.assertIn("vuln.py", files) + self.assertIn("vuln2.py", files) + self.assertIn("vuln_js.ts", files) + self.assertNotIn("safe.py", files) + self.assertNotIn("safe2.py", files) + + class SurfaceSinkTests(unittest.TestCase): # #1 + #7 def test_ssrf_outbound_var_arg_fires_not_literal(self): out = SurfaceExtractor().extract(repo({