Merge pull request github#13782 from jorgectf/jorgectf/shlex-quote

Python: Add `shlex.quote` as `py/shell-command-constructed-from-input` sanitizer

Author: yoff

python/ql/lib/change-notes/2023-07-20-shlex-quote-sanitizer.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added `shlex.quote` as a sanitizer for the `py/shell-command-constructed-from-input` query.

python/ql/lib/semmle/python/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll

@@ -9,6 +9,7 @@ private import semmle.python.dataflow.new.DataFlow
 private import semmle.python.dataflow.new.TaintTracking
 private import CommandInjectionCustomizations::CommandInjection as CommandInjection
 private import semmle.python.Concepts as Concepts
+private import semmle.python.ApiGraphs
 
 /**
  * Module containing sources, sinks, and sanitizers for shell command constructed from library input.
@@ -17,6 +18,9 @@ module UnsafeShellCommandConstruction {
   /** A source for shell command constructed from library input vulnerabilities. */
   abstract class Source extends DataFlow::Node { }
 
+  /** A sanitizer for shell command constructed from library input vulnerabilities. */
+  abstract class Sanitizer extends DataFlow::Node { }
+
   private import semmle.python.frameworks.Setuptools
 
   /** An input parameter to a gem seen as a source. */
@@ -156,4 +160,13 @@ module UnsafeShellCommandConstruction {
 
     override DataFlow::Node getStringConstruction() { result = formatCall }
   }
+
+  /**
+   * A call to `shlex.quote`, considered as a sanitizer.
+   */
+  class ShlexQuoteAsSanitizer extends Sanitizer, DataFlow::Node {
+    ShlexQuoteAsSanitizer() {
+      this = API::moduleImport("shlex").getMember("quote").getACall().getArg(0)
+    }
+  }
 }

python/ql/lib/semmle/python/security/dataflow/UnsafeShellCommandConstructionQuery.qll

@@ -24,7 +24,8 @@ class Configuration extends TaintTracking::Configuration {
   override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
 
   override predicate isSanitizer(DataFlow::Node node) {
-    node instanceof CommandInjection::Sanitizer // using all sanitizers from `rb/command-injection`
+    node instanceof Sanitizer or
+    node instanceof CommandInjection::Sanitizer // using all sanitizers from `py/command-injection`
   }
 
   // override to require the path doesn't have unmatched return steps

python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/src/unsafe_shell_test.py

@@ -46,4 +46,8 @@ def indirect(flag, x):
     subprocess.Popen("ping " + name, shell=unknownValue) # OK - shell assumed to be False
 
 def intentional(command): 
-    os.system("fish -ic " + command) # $result=OK - intentional
+    os.system("fish -ic " + command) # $result=OK - intentional
+
+import shlex
+def unsafe_shell_sanitized(name): 
+    os.system("ping " + shlex.quote(name)) # $result=OK - sanitized