Merge pull request #4700 from RasmusWL/python-add-code-injection-FP

Approved by tausbn

python/ql/src/semmle/python/Flow.qll

@@ -738,6 +738,7 @@ class ListNode extends SequenceNode {
   }
 }
 
+/** A control flow node corresponding to a set expression, such as `{ 1, 3, 5, 7, 9 }` */
 class SetNode extends ControlFlowNode {
   SetNode() { toAst(this) instanceof Set }
 
@@ -771,6 +772,25 @@ class DictNode extends ControlFlowNode {
   }
 }
 
+/**
+ * A control flow node corresponding to an iterable literal. Currently does not include
+ * dictionaries, use `DictNode` directly instead.
+ */
+class IterableNode extends ControlFlowNode {
+  IterableNode() {
+    this instanceof SequenceNode
+    or
+    this instanceof SetNode
+  }
+
+  /** Gets the control flow node for an element of this iterable. */
+  ControlFlowNode getAnElement() {
+    result = this.(SequenceNode).getAnElement()
+    or
+    result = this.(SetNode).getAnElement()
+  }
+}
+
 private AstNode assigned_value(Expr lhs) {
   /* lhs = result */
   exists(Assign a | a.getATarget() = lhs and result = a.getValue())

python/ql/src/semmle/python/dataflow/new/BarrierGuards.qll

@@ -0,0 +1,37 @@
+/** Provides commonly used BarrierGuards. */
+
+private import python
+private import semmle.python.dataflow.new.DataFlow
+
+/** A validation of unknown node by comparing with a constant string value. */
+class StringConstCompare extends DataFlow::BarrierGuard, CompareNode {
+  ControlFlowNode checked_node;
+  boolean safe_branch;
+
+  StringConstCompare() {
+    exists(StrConst str_const, Cmpop op |
+      op = any(Eq eq) and safe_branch = true
+      or
+      op = any(NotEq ne) and safe_branch = false
+    |
+      this.operands(str_const.getAFlowNode(), op, checked_node)
+      or
+      this.operands(checked_node, op, str_const.getAFlowNode())
+    )
+    or
+    exists(IterableNode str_const_iterable, Cmpop op |
+      op = any(In in_) and safe_branch = true
+      or
+      op = any(NotIn ni) and safe_branch = false
+    |
+      forall(ControlFlowNode elem | elem = str_const_iterable.getAnElement() |
+        elem.getNode() instanceof StrConst
+      ) and
+      this.operands(checked_node, op, str_const_iterable)
+    )
+  }
+
+  override predicate checks(ControlFlowNode node, boolean branch) {
+    node = checked_node and branch = safe_branch
+  }
+}

python/ql/src/semmle/python/security/dataflow/CodeInjection.qll

@@ -8,6 +8,7 @@ import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.TaintTracking
 import semmle.python.Concepts
 import semmle.python.dataflow.new.RemoteFlowSources
+import semmle.python.dataflow.new.BarrierGuards
 
 /**
  * A taint-tracking configuration for detecting code injection vulnerabilities.
@@ -18,4 +19,8 @@ class CodeInjectionConfiguration extends TaintTracking::Configuration {
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
   override predicate isSink(DataFlow::Node sink) { sink = any(CodeExecution e).getCode() }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof StringConstCompare
+  }
 }

python/ql/src/semmle/python/security/dataflow/CommandInjection.qll

@@ -8,6 +8,7 @@ import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.TaintTracking
 import semmle.python.Concepts
 import semmle.python.dataflow.new.RemoteFlowSources
+import semmle.python.dataflow.new.BarrierGuards
 
 /**
  * A taint-tracking configuration for detecting command injection vulnerabilities.
@@ -48,4 +49,8 @@ class CommandInjectionConfiguration extends TaintTracking::Configuration {
     // https://github.com/python/cpython/blob/fa7ce080175f65d678a7d5756c94f82887fc9803/Lib/subprocess.py#L341
     not sink.getScope().getEnclosingModule().getName() in ["os", "subprocess", "platform", "popen2"]
   }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof StringConstCompare
+  }
 }

python/ql/src/semmle/python/security/dataflow/PathInjection.qll

@@ -31,6 +31,7 @@ import semmle.python.dataflow.new.TaintTracking2
 import semmle.python.Concepts
 import semmle.python.dataflow.new.RemoteFlowSources
 import ChainedConfigs12
+import semmle.python.dataflow.new.BarrierGuards
 
 // ---------------------------------------------------------------------------
 // Case 1. The path is never normalized.
@@ -46,6 +47,10 @@ class PathNotNormalizedConfiguration extends TaintTracking::Configuration {
   }
 
   override predicate isSanitizer(DataFlow::Node node) { node instanceof Path::PathNormalization }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof StringConstCompare
+  }
 }
 
 /**
@@ -68,6 +73,10 @@ class FirstNormalizationConfiguration extends TaintTracking::Configuration {
   override predicate isSink(DataFlow::Node sink) { sink instanceof Path::PathNormalization }
 
   override predicate isSanitizerOut(DataFlow::Node node) { node instanceof Path::PathNormalization }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof StringConstCompare
+  }
 }
 
 /** Configuration to find paths from normalizations to sinks that do not go through a check. */
@@ -82,6 +91,8 @@ class NormalizedPathNotCheckedConfiguration extends TaintTracking2::Configuratio
 
   override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
     guard instanceof Path::SafeAccessCheck
+    or
+    guard instanceof StringConstCompare
   }
 }
 

python/ql/src/semmle/python/security/dataflow/ReflectedXSS.qll

@@ -8,6 +8,7 @@ import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.TaintTracking
 import semmle.python.Concepts
 import semmle.python.dataflow.new.RemoteFlowSources
+import semmle.python.dataflow.new.BarrierGuards
 
 /**
  * A taint-tracking configuration for detecting reflected server-side cross-site
@@ -24,4 +25,8 @@ class ReflectedXssConfiguration extends TaintTracking::Configuration {
       sink = response.getBody()
     )
   }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof StringConstCompare
+  }
 }

python/ql/src/semmle/python/security/dataflow/SqlInjection.qll

@@ -8,6 +8,7 @@ import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.TaintTracking
 import semmle.python.Concepts
 import semmle.python.dataflow.new.RemoteFlowSources
+import semmle.python.dataflow.new.BarrierGuards
 
 /**
  * A taint-tracking configuration for detecting SQL injection vulnerabilities.
@@ -18,4 +19,8 @@ class SQLInjectionConfiguration extends TaintTracking::Configuration {
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
   override predicate isSink(DataFlow::Node sink) { sink = any(SqlExecution e).getSql() }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof StringConstCompare
+  }
 }

python/ql/src/semmle/python/security/dataflow/UnsafeDeserialization.qll

@@ -8,6 +8,7 @@ import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.TaintTracking
 import semmle.python.Concepts
 import semmle.python.dataflow.new.RemoteFlowSources
+import semmle.python.dataflow.new.BarrierGuards
 
 /**
  * A taint-tracking configuration for detecting arbitrary code execution
@@ -24,4 +25,8 @@ class UnsafeDeserializationConfiguration extends TaintTracking::Configuration {
       sink = d.getAnInput()
     )
   }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof StringConstCompare
+  }
 }