Merge pull request #3429 from semgrep/update-subprocess-shell-rule

Update subprocess-shell-true and add fixtest

python/lang/security/audit/subprocess-shell-true.fixed.py

@@ -0,0 +1,20 @@
+import subprocess
+import sys
+
+# ok:subprocess-shell-true
+subprocess.call("echo 'hello'")
+
+# ok:subprocess-shell-true
+subprocess.call("grep -R {} .".format(sys.argv[1]))
+
+# ok:subprocess-shell-true
+subprocess.call("echo 'hello'", shell=True)
+
+# ruleid:subprocess-shell-true
+subprocess.call("grep -R {} .".format(sys.argv[1]), shell=False)
+
+# ruleid:subprocess-shell-true
+subprocess.call("grep -R {} .".format(sys.argv[1]), shell=False, cwd="/home/user")
+
+# ruleid:subprocess-shell-true
+subprocess.run("grep -R {} .".format(sys.argv[1]), shell=False)

python/lang/security/audit/subprocess-shell-true.yaml

@@ -1,18 +1,22 @@
 rules:
 - id: subprocess-shell-true
   patterns:
-  - pattern: subprocess.$FUNC(..., shell=True, ...)
+  - pattern: subprocess.$FUNC(..., shell=$TRUE, ...)
+  - metavariable-pattern:
+      metavariable: $TRUE
+      pattern: |
+        True 
   - pattern-not: subprocess.$FUNC("...", shell=True, ...)
+  - focus-metavariable: $TRUE
   message: >-
     Found 'subprocess' function '$FUNC' with 'shell=True'. This is dangerous because
     this call will spawn
     the command using a shell process. Doing so propagates current shell settings
     and variables, which
     makes it much easier for a malicious actor to execute commands. Use 'shell=False'
     instead.
-  fix-regex:
-    regex: (shell\s*=\s*)True
-    replacement: \1False
+  fix: |
+    False
   metadata:
     source-rule-url: https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html
     owasp:
@@ -29,7 +33,7 @@ rules:
     cwe2022-top25: true
     cwe2021-top25: true
     subcategory:
-    - vuln
+    - 'secure default'
     likelihood: HIGH
     impact: LOW
     confidence: MEDIUM