Merge branch 'main' of github.com:github/codeql into tausbn-python-add-source-nodes

python/change-notes/2020-12-03-model-realpath-abspath.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* Added modeling of `os.path.abspath` and `os.path.realpath` for Path Injection (py/path-injection).

python/change-notes/2020-12-09-add-sqlite3-model.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* Added model of `sqlite3` as SQL interface following PEP249, resulting in additional sinks for `py/sql-injection`.

python/change-notes/2020-12-14-add-PyMySQL-model.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* Added model of `PyMySQL` PyPI package as a SQL interface following PEP249, resulting in additional sinks for `py/sql-injection`.

python/ql/src/Security/CWE-020/IncompleteUrlSubstringSanitization.ql

@@ -17,9 +17,8 @@ private string commonTopLevelDomainRegex() { result = "com|org|edu|gov|uk|net|io
 
 predicate looksLikeUrl(StrConst s) {
   exists(string text | text = s.getText() |
-    text
-        .regexpMatch("(?i)([a-z]*:?//)?\\.?([a-z0-9-]+\\.)+(" + commonTopLevelDomainRegex() +
-            ")(:[0-9]+)?/?")
+    text.regexpMatch("(?i)([a-z]*:?//)?\\.?([a-z0-9-]+\\.)+(" + commonTopLevelDomainRegex() +
+        ")(:[0-9]+)?/?")
     or
     // target is a HTTP URL to a domain on any TLD
     text.regexpMatch("(?i)https?://([a-z0-9-]+\\.)+([a-z]+)(:[0-9]+)?/?")

python/ql/src/Security/CWE-022/PathInjection.qhelp

@@ -43,7 +43,8 @@ In the second example, it appears that the user is restricted to opening a file
 special characters. For example, the string <code>"../../../etc/passwd"</code> will result in the code
 reading the file located at <code>"/server/static/images/../../../etc/passwd"</code>, which is the system's
 password file. This file would then be sent back to the user, giving them access to all the
-system's passwords.
+system's passwords. Note that a user could also use an absolute path here, since the result of
+<code>os.path.join("/server/static/images/", "/etc/passwd")</code> is <code>"/etc/passwd"</code>.
 </p>
 
 <p>

python/ql/src/analysis/Consistency.ql

@@ -184,8 +184,7 @@ predicate ssa_consistency(string clsname, string problem, string what) {
   /* Minimality of phi nodes */
   exists(SsaVariable var |
     strictcount(var.getAPhiInput()) = 1 and
-    var
-        .getAPhiInput()
+    var.getAPhiInput()
         .getDefinition()
         .getBasicBlock()
         .strictlyDominates(var.getDefinition().getBasicBlock())

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/Frameworks.qll

@@ -7,8 +7,9 @@ private import semmle.python.frameworks.Django
 private import semmle.python.frameworks.Fabric
 private import semmle.python.frameworks.Flask
 private import semmle.python.frameworks.Invoke
-private import semmle.python.frameworks.MySQLdb
 private import semmle.python.frameworks.MysqlConnectorPython
+private import semmle.python.frameworks.MySQLdb
 private import semmle.python.frameworks.Psycopg2
+private import semmle.python.frameworks.PyMySQL
 private import semmle.python.frameworks.Stdlib
 private import semmle.python.frameworks.Yaml

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/dataflow/new/internal/TaintTrackingPrivate.qll

@@ -31,8 +31,6 @@ predicate localAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeT
   or
   stringManipulation(nodeFrom, nodeTo)
   or
-  jsonStep(nodeFrom, nodeTo)
-  or
   containerStep(nodeFrom, nodeTo)
   or
   copyStep(nodeFrom, nodeTo)
@@ -135,16 +133,6 @@ predicate stringManipulation(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeT
   // TODO: Handle functions in https://docs.python.org/3/library/binascii.html
 }
 
-/**
- * Holds if taint can flow from `nodeFrom` to `nodeTo` with a step related to JSON encoding/decoding.
- */
-predicate jsonStep(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeTo) {
-  exists(CallNode call | call = nodeTo.getNode() |
-    call.getFunction().(AttrNode).getObject(["load", "loads", "dumps"]).(NameNode).getId() = "json" and
-    call.getArg(0) = nodeFrom.getNode()
-  )
-}
-
 /**
  * Holds if taint can flow from `nodeFrom` to `nodeTo` with a step related to containers
  * (lists/sets/dictionaries): literals, constructor invocation, methods. Note that this

python/ql/src/semmle/python/dataflow/old/Implementation.qll

@@ -607,8 +607,7 @@ class TaintTrackingImplementation extends string {
     TaintTrackingNode src, DataFlow::Node node, TaintTrackingContext context, AttributePath path,
     TaintKind kind, string edgeLabel
   ) {
-    this
-        .(EssaTaintTracking)
+    this.(EssaTaintTracking)
         .taintedDefinition(src, node.asVariable().getDefinition(), context, path, kind) and
     edgeLabel = ""
   }

python/ql/src/semmle/python/filters/GeneratedCode.qll

@@ -95,8 +95,7 @@ private predicate dont_modify(File f) {
 private predicate auto_generated(File f) {
   exists(Comment c |
     c.getLocation().getFile() = f and
-    c
-        .getText()
+    c.getText()
         .regexpMatch("(?is)# *this +(code|file) +is +(auto(matically)?[ -]?generated|created automatically).*")
   )
 }

python/ql/src/semmle/python/frameworks/MySQLdb.qll

@@ -17,7 +17,7 @@ private import PEP249
  * - https://mysqlclient.readthedocs.io/index.html
  * - https://pypi.org/project/MySQL-python/
  */
-module MySQLdb {
+private module MySQLdb {
   // ---------------------------------------------------------------------------
   // MySQLdb
   // ---------------------------------------------------------------------------

python/ql/src/semmle/python/frameworks/MysqlConnectorPython.qll

@@ -17,7 +17,7 @@ private import PEP249
  * - https://dev.mysql.com/doc/connector-python/en/
  * - https://dev.mysql.com/doc/connector-python/en/connector-python-example-connecting.html
  */
-module MysqlConnectorPython {
+private module MysqlConnectorPython {
   // ---------------------------------------------------------------------------
   // mysql
   // ---------------------------------------------------------------------------

python/ql/src/semmle/python/frameworks/PEP249.qll

@@ -62,11 +62,11 @@ module Connection {
 }
 
 /**
- * Provides models for the `db.Connection.cursor` method.
+ * Provides models for the `cursor` method on a connection.
  * See https://www.python.org/dev/peps/pep-0249/#cursor.
  */
 module cursor {
-  /** Gets a reference to the `db.connection.cursor` method. */
+  /** Gets a reference to the `cursor` method on a connection. */
   private DataFlow::Node methodRef(DataFlow::TypeTracker t) {
     t.startInAttr("cursor") and
     result = Connection::instance()
@@ -74,10 +74,10 @@ module cursor {
     exists(DataFlow::TypeTracker t2 | result = methodRef(t2).track(t2, t))
   }
 
-  /** Gets a reference to the `db.connection.cursor` metod. */
+  /** Gets a reference to the `cursor` method on a connection. */
   DataFlow::Node methodRef() { result = methodRef(DataFlow::TypeTracker::end()) }
 
-  /** Gets a reference to a result of calling `db.connection.cursor`. */
+  /** Gets a reference to a result of calling the `cursor` method on a connection. */
   private DataFlow::Node methodResult(DataFlow::TypeTracker t) {
     t.start() and
     result.asCfgNode().(CallNode).getFunction() = methodRef().asCfgNode()
@@ -85,31 +85,40 @@ module cursor {
     exists(DataFlow::TypeTracker t2 | result = methodResult(t2).track(t2, t))
   }
 
-  /** Gets a reference to a result of calling `db.connection.cursor`. */
+  /** Gets a reference to a result of calling the `cursor` method on a connection. */
   DataFlow::Node methodResult() { result = methodResult(DataFlow::TypeTracker::end()) }
 }
 
 /**
- * Gets a reference to the `db.Connection.Cursor.execute` function.
+ * Gets a reference to the `execute` method on a cursor (or on a connection).
+ *
+ * Note: while `execute` method on a connection is not part of PEP249, if it is used, we
+ * recognize it as an alias for constructing a cursor and calling `execute` on it.
+ *
  * See https://www.python.org/dev/peps/pep-0249/#id15.
  */
 private DataFlow::Node execute(DataFlow::TypeTracker t) {
   t.startInAttr("execute") and
-  result = cursor::methodResult()
+  result in [cursor::methodResult(), Connection::instance()]
   or
   exists(DataFlow::TypeTracker t2 | result = execute(t2).track(t2, t))
 }
 
 /**
- * Gets a reference to the `db.Connection.Cursor.execute` function.
+ * Gets a reference to the `execute` method on a cursor (or on a connection).
+ *
+ * Note: while `execute` method on a connection is not part of PEP249, if it is used, we
+ * recognize it as an alias for constructing a cursor and calling `execute` on it.
+ *
  * See https://www.python.org/dev/peps/pep-0249/#id15.
  */
 DataFlow::Node execute() { result = execute(DataFlow::TypeTracker::end()) }
 
-private class DbConnectionExecute extends SqlExecution::Range, DataFlow::CfgNode {
+/** A call to the `execute` method on a cursor (or on a connection). */
+private class ExecuteCall extends SqlExecution::Range, DataFlow::CfgNode {
   override CallNode node;
 
-  DbConnectionExecute() { node.getFunction() = execute().asCfgNode() }
+  ExecuteCall() { node.getFunction() = execute().asCfgNode() }
 
   override DataFlow::Node getSql() {
     result.asCfgNode() in [node.getArg(0), node.getArgByName("sql")]

python/ql/src/semmle/python/frameworks/Psycopg2.qll

@@ -17,7 +17,7 @@ private import PEP249
  * - https://www.psycopg.org/docs/
  * - https://pypi.org/project/psycopg2/
  */
-module Psycopg2 {
+private module Psycopg2 {
   // ---------------------------------------------------------------------------
   // Psycopg
   // ---------------------------------------------------------------------------

python/ql/src/semmle/python/frameworks/PyMySQL.qll

@@ -0,0 +1,32 @@
+/**
+ * Provides classes modeling security-relevant aspects of the `PyMySQL` PyPI package.
+ * See https://pypi.org/project/PyMySQL/
+ */
+
+private import python
+private import semmle.python.dataflow.new.DataFlow
+private import semmle.python.dataflow.new.RemoteFlowSources
+private import semmle.python.Concepts
+private import PEP249
+
+/**
+ * Provides models for the `PyMySQL` PyPI package.
+ * See https://pypi.org/project/PyMySQL/
+ */
+private module PyMySQL {
+  /** Gets a reference to the `pymysql` module. */
+  private DataFlow::Node pymysql(DataFlow::TypeTracker t) {
+    t.start() and
+    result = DataFlow::importNode("pymysql")
+    or
+    exists(DataFlow::TypeTracker t2 | result = pymysql(t2).track(t2, t))
+  }
+
+  /** Gets a reference to the `pymysql` module. */
+  DataFlow::Node pymysql() { result = pymysql(DataFlow::TypeTracker::end()) }
+
+  /** PyMySQL implements PEP 249, providing ways to execute SQL statements against a database. */
+  class PyMySQLPEP249 extends PEP249Module {
+    PyMySQLPEP249() { this = pymysql() }
+  }
+}

python/ql/src/semmle/python/frameworks/Stdlib.qll

@@ -8,6 +8,7 @@ private import semmle.python.dataflow.new.DataFlow
 private import semmle.python.dataflow.new.TaintTracking
 private import semmle.python.dataflow.new.RemoteFlowSources
 private import semmle.python.Concepts
+private import PEP249
 
 /** Provides models for the Python standard library. */
 private module Stdlib {
@@ -91,7 +92,7 @@ private module Stdlib {
        * For example, using `attr_name = "join"` will get all uses of `os.path.join`.
        */
       private DataFlow::Node path_attr(DataFlow::TypeTracker t, string attr_name) {
-        attr_name in ["join", "normpath"] and
+        attr_name in ["join", "normpath", "realpath", "abspath"] and
         (
           t.start() and
           result = DataFlow::importNode("os.path." + attr_name)
@@ -157,6 +158,54 @@ private module Stdlib {
     }
   }
 
+  /**
+   * A call to `os.path.abspath`.
+   * See https://docs.python.org/3/library/os.path.html#os.path.abspath
+   */
+  private class OsPathAbspathCall extends Path::PathNormalization::Range, DataFlow::CfgNode {
+    override CallNode node;
+
+    OsPathAbspathCall() { node.getFunction() = os::path::path_attr("abspath").asCfgNode() }
+
+    DataFlow::Node getPathArg() {
+      result.asCfgNode() in [node.getArg(0), node.getArgByName("path")]
+    }
+  }
+
+  /** An additional taint step for calls to `os.path.abspath` */
+  private class OsPathAbspathCallAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
+    override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+      exists(OsPathAbspathCall call |
+        nodeTo = call and
+        nodeFrom = call.getPathArg()
+      )
+    }
+  }
+
+  /**
+   * A call to `os.path.realpath`.
+   * See https://docs.python.org/3/library/os.path.html#os.path.realpath
+   */
+  private class OsPathRealpathCall extends Path::PathNormalization::Range, DataFlow::CfgNode {
+    override CallNode node;
+
+    OsPathRealpathCall() { node.getFunction() = os::path::path_attr("realpath").asCfgNode() }
+
+    DataFlow::Node getPathArg() {
+      result.asCfgNode() in [node.getArg(0), node.getArgByName("path")]
+    }
+  }
+
+  /** An additional taint step for calls to `os.path.realpath` */
+  private class OsPathRealpathCallAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
+    override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+      exists(OsPathRealpathCall call |
+        nodeTo = call and
+        nodeFrom = call.getPathArg()
+      )
+    }
+  }
+
   /**
    * A call to `os.system`.
    * See https://docs.python.org/3/library/os.html#os.system
@@ -945,6 +994,116 @@ private module Stdlib {
   private DataFlow::Node io_attr(string attr_name) {
     result = io_attr(DataFlow::TypeTracker::end(), attr_name)
   }
+
+  // ---------------------------------------------------------------------------
+  // json
+  // ---------------------------------------------------------------------------
+  /** Gets a reference to the `json` module. */
+  private DataFlow::Node json(DataFlow::TypeTracker t) {
+    t.start() and
+    result = DataFlow::importNode("json")
+    or
+    exists(DataFlow::TypeTracker t2 | result = json(t2).track(t2, t))
+  }
+
+  /** Gets a reference to the `json` module. */
+  DataFlow::Node json() { result = json(DataFlow::TypeTracker::end()) }
+
+  /**
+   * Gets a reference to the attribute `attr_name` of the `json` module.
+   * WARNING: Only holds for a few predefined attributes.
+   */
+  private DataFlow::Node json_attr(DataFlow::TypeTracker t, string attr_name) {
+    attr_name in ["loads", "dumps"] and
+    (
+      t.start() and
+      result = DataFlow::importNode("json" + "." + attr_name)
+      or
+      t.startInAttr(attr_name) and
+      result = json()
+    )
+    or
+    // Due to bad performance when using normal setup with `json_attr(t2, attr_name).track(t2, t)`
+    // we have inlined that code and forced a join
+    exists(DataFlow::TypeTracker t2 |
+      exists(DataFlow::StepSummary summary |
+        json_attr_first_join(t2, attr_name, result, summary) and
+        t = t2.append(summary)
+      )
+    )
+  }
+
+  pragma[nomagic]
+  private predicate json_attr_first_join(
+    DataFlow::TypeTracker t2, string attr_name, DataFlow::Node res, DataFlow::StepSummary summary
+  ) {
+    DataFlow::StepSummary::step(json_attr(t2, attr_name), res, summary)
+  }
+
+  /**
+   * Gets a reference to the attribute `attr_name` of the `json` module.
+   * WARNING: Only holds for a few predefined attributes.
+   */
+  private DataFlow::Node json_attr(string attr_name) {
+    result = json_attr(DataFlow::TypeTracker::end(), attr_name)
+  }
+
+  /**
+   * A call to `json.loads`
+   * See https://docs.python.org/3/library/json.html#json.loads
+   */
+  private class JsonLoadsCall extends Decoding::Range, DataFlow::CfgNode {
+    override CallNode node;
+
+    JsonLoadsCall() { node.getFunction() = json_attr("loads").asCfgNode() }
+
+    override predicate mayExecuteInput() { none() }
+
+    override DataFlow::Node getAnInput() { result.asCfgNode() = node.getArg(0) }
+
+    override DataFlow::Node getOutput() { result = this }
+
+    override string getFormat() { result = "JSON" }
+  }
+
+  /**
+   * A call to `json.dumps`
+   * See https://docs.python.org/3/library/json.html#json.dumps
+   */
+  private class JsonDumpsCall extends Encoding::Range, DataFlow::CfgNode {
+    override CallNode node;
+
+    JsonDumpsCall() { node.getFunction() = json_attr("dumps").asCfgNode() }
+
+    override DataFlow::Node getAnInput() { result.asCfgNode() = node.getArg(0) }
+
+    override DataFlow::Node getOutput() { result = this }
+
+    override string getFormat() { result = "JSON" }
+  }
+
+  // ---------------------------------------------------------------------------
+  // sqlite3
+  // ---------------------------------------------------------------------------
+  /** Gets a reference to the `sqlite3` module. */
+  private DataFlow::Node sqlite3(DataFlow::TypeTracker t) {
+    t.start() and
+    result = DataFlow::importNode("sqlite3")
+    or
+    exists(DataFlow::TypeTracker t2 | result = sqlite3(t2).track(t2, t))
+  }
+
+  /** Gets a reference to the `sqlite3` module. */
+  DataFlow::Node sqlite3() { result = sqlite3(DataFlow::TypeTracker::end()) }
+
+  /**
+   * sqlite3 implements PEP 249, providing ways to execute SQL statements against a database.
+   *
+   * See https://devdocs.io/python~3.9/library/sqlite3
+   */
+  class Sqlite3 extends PEP249Module {
+    Sqlite3() { this = sqlite3() }
+  }
 }
 
 // ---------------------------------------------------------------------------

python/ql/src/semmle/python/objects/ObjectAPI.qll

@@ -84,8 +84,7 @@ class Value extends TObject {
   predicate hasLocationInfo(
     string filepath, int startline, int startcolumn, int endline, int endcolumn
   ) {
-    this
-        .(ObjectInternal)
+    this.(ObjectInternal)
         .getOrigin()
         .getLocation()
         .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)

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
+  }
 }

python/ql/src/semmle/python/types/Object.qll

@@ -75,8 +75,7 @@ class Object extends @py_object {
     string filepath, int startline, int startcolumn, int endline, int endcolumn
   ) {
     this.hasOrigin() and
-    this
-        .getOrigin()
+    this.getOrigin()
         .getLocation()
         .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
     or

python/ql/test/experimental/dataflow/tainttracking/commonSanitizer/TestTaint.expected

@@ -0,0 +1,27 @@
+| test_string_const_compare.py:16 | ok   | test_eq | ts |
+| test_string_const_compare.py:18 | ok   | test_eq | ts |
+| test_string_const_compare.py:20 | ok   | test_eq | ts |
+| test_string_const_compare.py:27 | ok   | test_eq_unsafe | ts |
+| test_string_const_compare.py:29 | ok   | test_eq_unsafe | ts |
+| test_string_const_compare.py:35 | fail | test_eq_with_or | ts |
+| test_string_const_compare.py:37 | ok   | test_eq_with_or | ts |
+| test_string_const_compare.py:43 | ok   | test_non_eq1 | ts |
+| test_string_const_compare.py:45 | ok   | test_non_eq1 | ts |
+| test_string_const_compare.py:51 | ok   | test_non_eq2 | ts |
+| test_string_const_compare.py:53 | fail | test_non_eq2 | ts |
+| test_string_const_compare.py:59 | ok   | test_in_list | ts |
+| test_string_const_compare.py:61 | ok   | test_in_list | ts |
+| test_string_const_compare.py:67 | ok   | test_in_tuple | ts |
+| test_string_const_compare.py:69 | ok   | test_in_tuple | ts |
+| test_string_const_compare.py:75 | ok   | test_in_set | ts |
+| test_string_const_compare.py:77 | ok   | test_in_set | ts |
+| test_string_const_compare.py:83 | ok   | test_in_unsafe1 | ts |
+| test_string_const_compare.py:85 | ok   | test_in_unsafe1 | ts |
+| test_string_const_compare.py:91 | ok   | test_in_unsafe2 | ts |
+| test_string_const_compare.py:93 | ok   | test_in_unsafe2 | ts |
+| test_string_const_compare.py:99 | ok   | test_not_in1 | ts |
+| test_string_const_compare.py:101 | ok   | test_not_in1 | ts |
+| test_string_const_compare.py:107 | ok   | test_not_in2 | ts |
+| test_string_const_compare.py:109 | fail | test_not_in2 | ts |
+| test_string_const_compare.py:119 | fail | test_eq_thorugh_func | ts |
+| test_string_const_compare.py:121 | ok   | test_eq_thorugh_func | ts |

python/ql/test/experimental/dataflow/tainttracking/commonSanitizer/TestTaint.ql

@@ -0,0 +1,8 @@
+import experimental.dataflow.tainttracking.TestTaintLib
+import semmle.python.dataflow.new.BarrierGuards
+
+class CustomSanitizerOverrides extends TestTaintTrackingConfiguration {
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof StringConstCompare
+  }
+}

python/ql/test/experimental/dataflow/tainttracking/commonSanitizer/test_string_const_compare.py

@@ -0,0 +1,138 @@
+# Add taintlib to PATH so it can be imported during runtime without any hassle
+import sys; import os; sys.path.append(os.path.dirname(os.path.dirname((__file__))))
+from taintlib import *
+
+# This has no runtime impact, but allows autocomplete to work
+from typing import TYPE_CHECKING
+if TYPE_CHECKING:
+    from ..taintlib import *
+
+
+# Actual tests
+
+def test_eq():
+    ts = TAINTED_STRING
+    if ts == "safe":
+        ensure_not_tainted(ts)
+    else:
+        ensure_tainted(ts)
+    # ts should still be tainted after exiting the if block
+    ensure_tainted(ts)
+
+
+def test_eq_unsafe(x="foo"):
+    """This test-case might seem strange, but it was a FP in our old points-to based analysis."""
+    ts = TAINTED_STRING
+    if ts == ts:
+        ensure_tainted(ts)
+    if ts == x:
+        ensure_tainted(ts)
+
+
+def test_eq_with_or():
+    ts = TAINTED_STRING
+    if ts == "safe" or ts == "also_safe":
+        ensure_not_tainted(ts)
+    else:
+        ensure_tainted(ts)
+
+
+def test_non_eq1():
+    ts = TAINTED_STRING
+    if ts != "safe":
+        ensure_tainted(ts)
+    else:
+        ensure_not_tainted(ts)
+
+
+def test_non_eq2():
+    ts = TAINTED_STRING
+    if not ts == "safe":
+        ensure_tainted(ts)
+    else:
+        ensure_not_tainted(ts)
+
+
+def test_in_list():
+    ts = TAINTED_STRING
+    if ts in ["safe", "also_safe"]:
+        ensure_not_tainted(ts)
+    else:
+        ensure_tainted(ts)
+
+
+def test_in_tuple():
+    ts = TAINTED_STRING
+    if ts in ("safe", "also_safe"):
+        ensure_not_tainted(ts)
+    else:
+        ensure_tainted(ts)
+
+
+def test_in_set():
+    ts = TAINTED_STRING
+    if ts in {"safe", "also_safe"}:
+        ensure_not_tainted(ts)
+    else:
+        ensure_tainted(ts)
+
+
+def test_in_unsafe1(xs):
+    ts = TAINTED_STRING
+    if ts in xs:
+        ensure_tainted(ts)
+    else:
+        ensure_tainted(ts)
+
+
+def test_in_unsafe2(x):
+    ts = TAINTED_STRING
+    if ts in ["safe", x]:
+        ensure_tainted(ts)
+    else:
+        ensure_tainted(ts)
+
+
+def test_not_in1():
+    ts = TAINTED_STRING
+    if ts not in ["safe", "also_safe"]:
+        ensure_tainted(ts)
+    else:
+        ensure_not_tainted(ts)
+
+
+def test_not_in2():
+    ts = TAINTED_STRING
+    if not ts in ["safe", "also_safe"]:
+        ensure_tainted(ts)
+    else:
+        ensure_not_tainted(ts)
+
+
+def is_safe(x):
+    return x == "safe"
+
+
+def test_eq_thorugh_func():
+    ts = TAINTED_STRING
+    if is_safe(ts):
+        ensure_not_tainted(ts)
+    else:
+        ensure_tainted(ts)
+
+
+# Make tests runable
+
+test_eq()
+test_eq_unsafe()
+test_eq_with_or()
+test_non_eq1()
+test_non_eq2()
+test_in_list()
+test_in_tuple()
+test_in_set()
+test_in_unsafe1(["unsafe", "foo"])
+test_in_unsafe2("unsafe")
+test_not_in1()
+test_not_in2()
+test_eq_thorugh_func()

python/ql/test/experimental/dataflow/tainttracking/customSanitizer/TestTaint.expected

@@ -4,8 +4,47 @@ test_taint
 | test.py:38 | ok   | test_custom_sanitizer_guard | s |
 | test.py:40 | ok   | test_custom_sanitizer_guard | s |
 | test.py:51 | ok   | test_escape | s2 |
+| test_logical.py:30 | ok   | test_basic | s |
+| test_logical.py:32 | ok   | test_basic | s |
+| test_logical.py:35 | ok   | test_basic | s |
+| test_logical.py:37 | fail | test_basic | s |
+| test_logical.py:45 | ok   | test_or | s |
+| test_logical.py:47 | ok   | test_or | s |
+| test_logical.py:51 | ok   | test_or | s |
+| test_logical.py:53 | ok   | test_or | s |
+| test_logical.py:57 | ok   | test_or | s |
+| test_logical.py:59 | ok   | test_or | s |
+| test_logical.py:67 | ok   | test_and | s |
+| test_logical.py:69 | ok   | test_and | s |
+| test_logical.py:73 | ok   | test_and | s |
+| test_logical.py:75 | ok   | test_and | s |
+| test_logical.py:79 | ok   | test_and | s |
+| test_logical.py:81 | fail | test_and | s |
+| test_logical.py:89 | fail | test_tricky | s |
+| test_logical.py:93 | fail | test_tricky | s_ |
+| test_logical.py:100 | fail | test_nesting_not | s |
+| test_logical.py:102 | ok   | test_nesting_not | s |
+| test_logical.py:105 | ok   | test_nesting_not | s |
+| test_logical.py:107 | fail | test_nesting_not | s |
+| test_logical.py:116 | ok   | test_nesting_not_with_and_true | s |
+| test_logical.py:118 | ok   | test_nesting_not_with_and_true | s |
+| test_logical.py:121 | ok   | test_nesting_not_with_and_true | s |
+| test_logical.py:123 | ok   | test_nesting_not_with_and_true | s |
+| test_logical.py:126 | ok   | test_nesting_not_with_and_true | s |
+| test_logical.py:128 | ok   | test_nesting_not_with_and_true | s |
+| test_logical.py:137 | fail | test_with_return | s |
+| test_logical.py:146 | fail | test_with_exception | s |
 isSanitizer
 | TestTaintTrackingConfiguration | test.py:21:39:21:39 | ControlFlowNode for s |
 | TestTaintTrackingConfiguration | test.py:50:10:50:29 | ControlFlowNode for emulated_escaping() |
 isSanitizerGuard
 | TestTaintTrackingConfiguration | test.py:35:8:35:26 | ControlFlowNode for emulated_is_safe() |
+| TestTaintTrackingConfiguration | test_logical.py:29:8:29:17 | ControlFlowNode for is_safe() |
+| TestTaintTrackingConfiguration | test_logical.py:44:8:44:17 | ControlFlowNode for is_safe() |
+| TestTaintTrackingConfiguration | test_logical.py:50:12:50:21 | ControlFlowNode for is_safe() |
+| TestTaintTrackingConfiguration | test_logical.py:66:8:66:17 | ControlFlowNode for is_safe() |
+| TestTaintTrackingConfiguration | test_logical.py:72:12:72:21 | ControlFlowNode for is_safe() |
+| TestTaintTrackingConfiguration | test_logical.py:92:8:92:17 | ControlFlowNode for is_safe() |
+| TestTaintTrackingConfiguration | test_logical.py:115:12:115:21 | ControlFlowNode for is_safe() |
+| TestTaintTrackingConfiguration | test_logical.py:120:16:120:25 | ControlFlowNode for is_safe() |
+| TestTaintTrackingConfiguration | test_logical.py:125:20:125:29 | ControlFlowNode for is_safe() |

python/ql/test/experimental/dataflow/tainttracking/customSanitizer/TestTaint.ql

@@ -1,7 +1,9 @@
 import experimental.dataflow.tainttracking.TestTaintLib
 
 class IsSafeCheck extends DataFlow::BarrierGuard {
-  IsSafeCheck() { this.(CallNode).getNode().getFunc().(Name).getId() = "emulated_is_safe" }
+  IsSafeCheck() {
+    this.(CallNode).getNode().getFunc().(Name).getId() in ["is_safe", "emulated_is_safe"]
+  }
 
   override predicate checks(ControlFlowNode node, boolean branch) {
     node = this.(CallNode).getAnArg() and

python/ql/test/experimental/dataflow/tainttracking/customSanitizer/test_logical.py

@@ -128,6 +128,23 @@ def test_nesting_not_with_and_true():
         ensure_not_tainted(s)
 
 
+def test_with_return():
+    s = TAINTED_STRING
+
+    if not is_safe(s):
+        return
+
+    ensure_not_tainted(s)
+
+
+def test_with_exception():
+    s = TAINTED_STRING
+
+    if not is_safe(s):
+        raise Exception("unsafe")
+
+    ensure_not_tainted(s)
+
 # Make tests runable
 
 test_basic()
@@ -136,3 +153,8 @@ test_and()
 test_tricky()
 test_nesting_not()
 test_nesting_not_with_and_true()
+test_with_return()
+try:
+    test_with_exception()
+except:
+    pass

python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/TestTaint.expected

@@ -68,9 +68,9 @@
 | test_json.py:27 | ok   | test | json.loads(..) |
 | test_json.py:34 | fail | test | tainted_filelike |
 | test_json.py:35 | fail | test | json.load(..) |
-| test_json.py:48 | fail | non_syntacical | dumps(..) |
-| test_json.py:49 | fail | non_syntacical | dumps_alias(..) |
-| test_json.py:50 | fail | non_syntacical | loads(..) |
+| test_json.py:48 | ok   | non_syntacical | dumps(..) |
+| test_json.py:49 | ok   | non_syntacical | dumps_alias(..) |
+| test_json.py:50 | ok   | non_syntacical | loads(..) |
 | test_json.py:57 | fail | non_syntacical | tainted_filelike |
 | test_json.py:58 | fail | non_syntacical | load(..) |
 | test_string.py:25 | ok   | str_operations | ts |

python/ql/test/experimental/dataflow/tainttracking/defaultSanitizer/TestTaint.expected

@@ -1,37 +0,0 @@
-| test_logical.py:30 | fail | test_basic | s |
-| test_logical.py:32 | ok   | test_basic | s |
-| test_logical.py:35 | ok   | test_basic | s |
-| test_logical.py:37 | fail | test_basic | s |
-| test_logical.py:45 | ok   | test_or | s |
-| test_logical.py:47 | ok   | test_or | s |
-| test_logical.py:51 | ok   | test_or | s |
-| test_logical.py:53 | ok   | test_or | s |
-| test_logical.py:57 | ok   | test_or | s |
-| test_logical.py:59 | ok   | test_or | s |
-| test_logical.py:67 | fail | test_and | s |
-| test_logical.py:69 | ok   | test_and | s |
-| test_logical.py:73 | ok   | test_and | s |
-| test_logical.py:75 | fail | test_and | s |
-| test_logical.py:79 | ok   | test_and | s |
-| test_logical.py:81 | fail | test_and | s |
-| test_logical.py:89 | fail | test_tricky | s |
-| test_logical.py:93 | fail | test_tricky | s_ |
-| test_logical.py:100 | fail | test_nesting_not | s |
-| test_logical.py:102 | ok   | test_nesting_not | s |
-| test_logical.py:105 | ok   | test_nesting_not | s |
-| test_logical.py:107 | fail | test_nesting_not | s |
-| test_logical.py:116 | ok   | test_nesting_not_with_and_true | s |
-| test_logical.py:118 | fail | test_nesting_not_with_and_true | s |
-| test_logical.py:121 | fail | test_nesting_not_with_and_true | s |
-| test_logical.py:123 | ok   | test_nesting_not_with_and_true | s |
-| test_logical.py:126 | ok   | test_nesting_not_with_and_true | s |
-| test_logical.py:128 | fail | test_nesting_not_with_and_true | s |
-| test_string_eq.py:16 | fail | const_eq_clears_taint | ts |
-| test_string_eq.py:18 | ok   | const_eq_clears_taint | ts |
-| test_string_eq.py:20 | ok   | const_eq_clears_taint | ts |
-| test_string_eq.py:27 | fail | const_eq_clears_taint2 | ts |
-| test_string_eq.py:33 | ok   | non_const_eq_preserves_taint | ts |
-| test_string_eq.py:35 | ok   | non_const_eq_preserves_taint | ts |
-| test_string_eq.py:45 | fail | const_eq_through_func | ts |
-| test_string_eq.py:47 | ok   | const_eq_through_func | ts |
-| test_string_eq.py:49 | ok   | const_eq_through_func | ts |

python/ql/test/experimental/dataflow/tainttracking/defaultSanitizer/TestTaint.ql

@@ -1 +0,0 @@
-import experimental.dataflow.tainttracking.TestTaintLib

python/ql/test/experimental/dataflow/tainttracking/defaultSanitizer/test_string_eq.py

@@ -1,56 +0,0 @@
-# Add taintlib to PATH so it can be imported during runtime without any hassle
-import sys; import os; sys.path.append(os.path.dirname(os.path.dirname((__file__))))
-from taintlib import *
-
-# This has no runtime impact, but allows autocomplete to work
-from typing import TYPE_CHECKING
-if TYPE_CHECKING:
-    from ..taintlib import *
-
-
-# Actual tests
-
-def const_eq_clears_taint():
-    ts = TAINTED_STRING
-    if ts == "safe":
-        ensure_not_tainted(ts)
-    else:
-        ensure_tainted(ts)
-    # ts should still be tainted after exiting the if block
-    ensure_tainted(ts)
-
-
-def const_eq_clears_taint2():
-    ts = TAINTED_STRING
-    if ts != "safe":
-        return
-    ensure_not_tainted(ts)
-
-
-def non_const_eq_preserves_taint(x="foo"):
-    ts = TAINTED_STRING
-    if ts == ts:
-        ensure_tainted(ts)
-    if ts == x:
-        ensure_tainted(ts)
-
-
-def is_safe(x):
-    return x == "safe"
-
-
-def const_eq_through_func():
-    ts = TAINTED_STRING
-    if is_safe(ts):
-        ensure_not_tainted(ts)
-    else:
-        ensure_tainted(ts)
-    # ts should still be tainted after exiting the if block
-    ensure_tainted(ts)
-
-
-# Make tests runable
-
-const_eq_clears_taint()
-const_eq_clears_taint2()
-non_const_eq_preserves_taint()

python/ql/test/experimental/library-tests/frameworks/pymysql/ConceptsTest.ql

@@ -0,0 +1,2 @@
+import python
+import experimental.meta.ConceptsTest

python/ql/test/experimental/library-tests/frameworks/pymysql/pep249.py

@@ -0,0 +1,5 @@
+import pymysql
+connection = pymysql.connect(host="localhost", user="user", password="passwd")
+
+cursor = connection.cursor()
+cursor.execute("some sql", (42,))  # $ getSql="some sql"

python/ql/test/experimental/library-tests/frameworks/stdlib/pep249.py

@@ -0,0 +1,8 @@
+import sqlite3
+db = sqlite3.connect("example.db")
+
+# non standard
+db.execute("some sql", (42,))  # $ getSql="some sql"
+
+cursor = db.cursor()
+cursor.execute("some sql", (42,))  # $ getSql="some sql"

python/ql/test/library-tests/regex/Mode.expected

@@ -7,4 +7,5 @@
 | 50 | VERBOSE |
 | 51 | UNICODE |
 | 52 | UNICODE |
-| 64 | MULTILINE |
+| 56 | VERBOSE |
+| 68 | MULTILINE |

python/ql/test/library-tests/regex/test.py

@@ -1,5 +1,5 @@
 import re
-#            0123456789ABCDEF  
+#            0123456789ABCDEF
 re.compile(r'012345678')
 re.compile(r'(\033|~{)')
 re.compile(r'\A[+-]?\d+')
@@ -50,6 +50,10 @@ re.compile("", re.VERBOSE|re.DOTALL)
 re.compile("", flags=re.VERBOSE|re.IGNORECASE)
 re.search("", None, re.UNICODE)
 x = re.search("", flags=re.UNICODE)
+# using addition for flags was reported as FP in https://github.com/github/codeql/issues/4707
+re.compile("", re.VERBOSE+re.DOTALL) # TODO: Currently not recognized with Mode.ql
+# re.X is an alias for re.VERBOSE
+re.compile("", re.X)
 
 #empty choice
 re.compile(r'|x')

python/ql/test/query-tests/Security/CWE-022-PathInjection/PathInjection.expected

@@ -1,10 +1,23 @@
 edges
-| path_injection.py:9:12:9:23 | ControlFlowNode for Attribute | path_injection.py:10:14:10:44 | ControlFlowNode for Attribute() |
-| path_injection.py:15:12:15:23 | ControlFlowNode for Attribute | path_injection.py:16:13:16:61 | ControlFlowNode for Attribute() |
-| path_injection.py:16:13:16:61 | ControlFlowNode for Attribute() | path_injection.py:17:14:17:18 | ControlFlowNode for npath |
-| path_injection.py:24:12:24:23 | ControlFlowNode for Attribute | path_injection.py:25:13:25:61 | ControlFlowNode for Attribute() |
-| path_injection.py:25:13:25:61 | ControlFlowNode for Attribute() | path_injection.py:28:14:28:18 | ControlFlowNode for npath |
-| path_injection.py:33:12:33:23 | ControlFlowNode for Attribute | path_injection.py:34:13:34:61 | ControlFlowNode for Attribute() |
+| path_injection.py:12:16:12:27 | ControlFlowNode for Attribute | path_injection.py:13:14:13:47 | ControlFlowNode for Attribute() |
+| path_injection.py:19:16:19:27 | ControlFlowNode for Attribute | path_injection.py:20:13:20:64 | ControlFlowNode for Attribute() |
+| path_injection.py:20:13:20:64 | ControlFlowNode for Attribute() | path_injection.py:21:14:21:18 | ControlFlowNode for npath |
+| path_injection.py:27:16:27:27 | ControlFlowNode for Attribute | path_injection.py:28:13:28:64 | ControlFlowNode for Attribute() |
+| path_injection.py:28:13:28:64 | ControlFlowNode for Attribute() | path_injection.py:31:14:31:18 | ControlFlowNode for npath |
+| path_injection.py:37:16:37:27 | ControlFlowNode for Attribute | path_injection.py:38:13:38:64 | ControlFlowNode for Attribute() |
+| path_injection.py:46:16:46:27 | ControlFlowNode for Attribute | path_injection.py:47:13:47:64 | ControlFlowNode for Attribute() |
+| path_injection.py:47:13:47:64 | ControlFlowNode for Attribute() | path_injection.py:48:14:48:18 | ControlFlowNode for npath |
+| path_injection.py:54:16:54:27 | ControlFlowNode for Attribute | path_injection.py:55:13:55:64 | ControlFlowNode for Attribute() |
+| path_injection.py:63:16:63:27 | ControlFlowNode for Attribute | path_injection.py:64:13:64:63 | ControlFlowNode for Attribute() |
+| path_injection.py:64:13:64:63 | ControlFlowNode for Attribute() | path_injection.py:65:14:65:18 | ControlFlowNode for npath |
+| path_injection.py:71:16:71:27 | ControlFlowNode for Attribute | path_injection.py:72:13:72:63 | ControlFlowNode for Attribute() |
+| path_injection.py:78:20:78:25 | ControlFlowNode for foo_id | path_injection.py:81:14:81:17 | ControlFlowNode for path |
+| path_injection.py:85:20:85:22 | ControlFlowNode for foo | path_injection.py:89:14:89:17 | ControlFlowNode for path |
+| path_injection.py:94:16:94:27 | ControlFlowNode for Attribute | path_injection.py:100:14:100:17 | ControlFlowNode for path |
+| path_injection.py:105:16:105:27 | ControlFlowNode for Attribute | path_injection.py:111:14:111:17 | ControlFlowNode for path |
+| path_injection.py:116:16:116:27 | ControlFlowNode for Attribute | path_injection.py:119:14:119:22 | ControlFlowNode for sanitized |
+| path_injection.py:125:16:125:27 | ControlFlowNode for Attribute | path_injection.py:127:30:127:51 | ControlFlowNode for Attribute() |
+| path_injection.py:125:16:125:27 | ControlFlowNode for Attribute | path_injection.py:129:14:129:17 | ControlFlowNode for path |
 | test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:9:12:9:39 | ControlFlowNode for Attribute() |
 | test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:9:12:9:39 | ControlFlowNode for Attribute() |
 | test.py:9:12:9:39 | ControlFlowNode for Attribute() | test.py:18:9:18:16 | ControlFlowNode for source() |
@@ -39,16 +52,40 @@ edges
 | test_chaining.py:41:9:41:16 | ControlFlowNode for source() | test_chaining.py:42:9:42:19 | ControlFlowNode for normpath() |
 | test_chaining.py:44:13:44:23 | ControlFlowNode for normpath() | test_chaining.py:45:14:45:14 | ControlFlowNode for z |
 nodes
-| path_injection.py:9:12:9:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
-| path_injection.py:10:14:10:44 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
-| path_injection.py:15:12:15:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
-| path_injection.py:16:13:16:61 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
-| path_injection.py:17:14:17:18 | ControlFlowNode for npath | semmle.label | ControlFlowNode for npath |
-| path_injection.py:24:12:24:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
-| path_injection.py:25:13:25:61 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
-| path_injection.py:28:14:28:18 | ControlFlowNode for npath | semmle.label | ControlFlowNode for npath |
-| path_injection.py:33:12:33:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
-| path_injection.py:34:13:34:61 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| path_injection.py:12:16:12:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:13:14:13:47 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| path_injection.py:19:16:19:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:20:13:20:64 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| path_injection.py:21:14:21:18 | ControlFlowNode for npath | semmle.label | ControlFlowNode for npath |
+| path_injection.py:27:16:27:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:28:13:28:64 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| path_injection.py:31:14:31:18 | ControlFlowNode for npath | semmle.label | ControlFlowNode for npath |
+| path_injection.py:37:16:37:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:38:13:38:64 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| path_injection.py:46:16:46:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:47:13:47:64 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| path_injection.py:48:14:48:18 | ControlFlowNode for npath | semmle.label | ControlFlowNode for npath |
+| path_injection.py:54:16:54:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:55:13:55:64 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| path_injection.py:63:16:63:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:64:13:64:63 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| path_injection.py:65:14:65:18 | ControlFlowNode for npath | semmle.label | ControlFlowNode for npath |
+| path_injection.py:71:16:71:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:72:13:72:63 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| path_injection.py:78:20:78:25 | ControlFlowNode for foo_id | semmle.label | ControlFlowNode for foo_id |
+| path_injection.py:81:14:81:17 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
+| path_injection.py:85:20:85:22 | ControlFlowNode for foo | semmle.label | ControlFlowNode for foo |
+| path_injection.py:89:14:89:17 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
+| path_injection.py:94:16:94:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:100:14:100:17 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
+| path_injection.py:105:16:105:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:111:14:111:17 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
+| path_injection.py:116:16:116:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:119:14:119:22 | ControlFlowNode for sanitized | semmle.label | ControlFlowNode for sanitized |
+| path_injection.py:125:16:125:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:125:16:125:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:127:30:127:51 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| path_injection.py:129:14:129:17 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
 | test.py:9:12:9:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
 | test.py:9:12:9:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
 | test.py:9:12:9:39 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
@@ -84,9 +121,17 @@ nodes
 | test_chaining.py:44:13:44:23 | ControlFlowNode for normpath() | semmle.label | ControlFlowNode for normpath() |
 | test_chaining.py:45:14:45:14 | ControlFlowNode for z | semmle.label | ControlFlowNode for z |
 #select
-| path_injection.py:10:14:10:44 | ControlFlowNode for Attribute() | path_injection.py:9:12:9:23 | ControlFlowNode for Attribute | path_injection.py:10:14:10:44 | ControlFlowNode for Attribute() | This path depends on $@. | path_injection.py:9:12:9:23 | ControlFlowNode for Attribute | a user-provided value |
-| path_injection.py:17:14:17:18 | ControlFlowNode for npath | path_injection.py:15:12:15:23 | ControlFlowNode for Attribute | path_injection.py:17:14:17:18 | ControlFlowNode for npath | This path depends on $@. | path_injection.py:15:12:15:23 | ControlFlowNode for Attribute | a user-provided value |
-| path_injection.py:28:14:28:18 | ControlFlowNode for npath | path_injection.py:24:12:24:23 | ControlFlowNode for Attribute | path_injection.py:28:14:28:18 | ControlFlowNode for npath | This path depends on $@. | path_injection.py:24:12:24:23 | ControlFlowNode for Attribute | a user-provided value |
+| path_injection.py:13:14:13:47 | ControlFlowNode for Attribute() | path_injection.py:12:16:12:27 | ControlFlowNode for Attribute | path_injection.py:13:14:13:47 | ControlFlowNode for Attribute() | This path depends on $@. | path_injection.py:12:16:12:27 | ControlFlowNode for Attribute | a user-provided value |
+| path_injection.py:21:14:21:18 | ControlFlowNode for npath | path_injection.py:19:16:19:27 | ControlFlowNode for Attribute | path_injection.py:21:14:21:18 | ControlFlowNode for npath | This path depends on $@. | path_injection.py:19:16:19:27 | ControlFlowNode for Attribute | a user-provided value |
+| path_injection.py:31:14:31:18 | ControlFlowNode for npath | path_injection.py:27:16:27:27 | ControlFlowNode for Attribute | path_injection.py:31:14:31:18 | ControlFlowNode for npath | This path depends on $@. | path_injection.py:27:16:27:27 | ControlFlowNode for Attribute | a user-provided value |
+| path_injection.py:48:14:48:18 | ControlFlowNode for npath | path_injection.py:46:16:46:27 | ControlFlowNode for Attribute | path_injection.py:48:14:48:18 | ControlFlowNode for npath | This path depends on $@. | path_injection.py:46:16:46:27 | ControlFlowNode for Attribute | a user-provided value |
+| path_injection.py:65:14:65:18 | ControlFlowNode for npath | path_injection.py:63:16:63:27 | ControlFlowNode for Attribute | path_injection.py:65:14:65:18 | ControlFlowNode for npath | This path depends on $@. | path_injection.py:63:16:63:27 | ControlFlowNode for Attribute | a user-provided value |
+| path_injection.py:81:14:81:17 | ControlFlowNode for path | path_injection.py:78:20:78:25 | ControlFlowNode for foo_id | path_injection.py:81:14:81:17 | ControlFlowNode for path | This path depends on $@. | path_injection.py:78:20:78:25 | ControlFlowNode for foo_id | a user-provided value |
+| path_injection.py:89:14:89:17 | ControlFlowNode for path | path_injection.py:85:20:85:22 | ControlFlowNode for foo | path_injection.py:89:14:89:17 | ControlFlowNode for path | This path depends on $@. | path_injection.py:85:20:85:22 | ControlFlowNode for foo | a user-provided value |
+| path_injection.py:100:14:100:17 | ControlFlowNode for path | path_injection.py:94:16:94:27 | ControlFlowNode for Attribute | path_injection.py:100:14:100:17 | ControlFlowNode for path | This path depends on $@. | path_injection.py:94:16:94:27 | ControlFlowNode for Attribute | a user-provided value |
+| path_injection.py:111:14:111:17 | ControlFlowNode for path | path_injection.py:105:16:105:27 | ControlFlowNode for Attribute | path_injection.py:111:14:111:17 | ControlFlowNode for path | This path depends on $@. | path_injection.py:105:16:105:27 | ControlFlowNode for Attribute | a user-provided value |
+| path_injection.py:119:14:119:22 | ControlFlowNode for sanitized | path_injection.py:116:16:116:27 | ControlFlowNode for Attribute | path_injection.py:119:14:119:22 | ControlFlowNode for sanitized | This path depends on $@. | path_injection.py:116:16:116:27 | ControlFlowNode for Attribute | a user-provided value |
+| path_injection.py:129:14:129:17 | ControlFlowNode for path | path_injection.py:125:16:125:27 | ControlFlowNode for Attribute | path_injection.py:129:14:129:17 | ControlFlowNode for path | This path depends on $@. | path_injection.py:125:16:125:27 | ControlFlowNode for Attribute | a user-provided value |
 | test.py:19:10:19:10 | ControlFlowNode for x | test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:19:10:19:10 | ControlFlowNode for x | This path depends on $@. | test.py:9:12:9:23 | ControlFlowNode for Attribute | a user-provided value |
 | test.py:26:10:26:10 | ControlFlowNode for y | test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:26:10:26:10 | ControlFlowNode for y | This path depends on $@. | test.py:9:12:9:23 | ControlFlowNode for Attribute | a user-provided value |
 | test.py:33:14:33:14 | ControlFlowNode for x | test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:33:14:33:14 | ControlFlowNode for x | This path depends on $@. | test.py:9:12:9:23 | ControlFlowNode for Attribute | a user-provided value |

python/ql/test/query-tests/Security/CWE-022-PathInjection/path_injection.py

@@ -1,36 +1,129 @@
-
-import os.path
+import os
 
 from flask import Flask, request
 app = Flask(__name__)
 
+
+STATIC_DIR = "/server/static/"
+
+
 @app.route("/path1")
 def path_injection():
-    path = request.args.get('path', '')
-    f = open(os.path.join(os.getcwd(), path))
+    filename = request.args.get('filename', '')
+    f = open(os.path.join(STATIC_DIR, filename)) # NOT OK
+
 
 @app.route("/path2")
 def path_injection():
     # Normalized, but not checked
-    path = request.args.get('path', '')
-    npath = os.path.normpath(os.path.join(os.getcwd(), path))
-    f = open(npath)  # Path not found
+    filename = request.args.get('filename', '')
+    npath = os.path.normpath(os.path.join(STATIC_DIR, filename))
+    f = open(npath)  # NOT OK
 
-SAFE = "/tmp/scratch_area/"
 
 @app.route("/path3")
-def safe_path():
-    # Normalized, but check doesn't reach open().
-    path = request.args.get('path', '')
-    npath = os.path.normpath(os.path.join(os.getcwd(), path))
-    if npath.startswith(SAFE):
+def unsafe_path_normpath():
+    # Normalized, but `open()` is not guarded by `startswith` check
+    filename = request.args.get('filename', '')
+    npath = os.path.normpath(os.path.join(STATIC_DIR, filename))
+    if npath.startswith(STATIC_DIR):
         pass
-    f = open(npath)  # Path not found
+    f = open(npath)  # NOT OK
+
 
 @app.route("/path4")
-def safe_path():
+def safe_path_normpath():
     # Normalized, and checked properly
-    path = request.args.get('path', '')
-    npath = os.path.normpath(os.path.join(os.getcwd(), path))
-    if npath.startswith(SAFE):
-        f = open(npath)
+    filename = request.args.get('filename', '')
+    npath = os.path.normpath(os.path.join(STATIC_DIR, filename))
+    if npath.startswith(STATIC_DIR):
+        f = open(npath)  # OK
+
+
+@app.route("/path5")
+def unsafe_path_realpath():
+    # Normalized (by `realpath` that also follows symlinks), but not checked properly
+    filename = request.args.get('filename', '')
+    npath = os.path.realpath(os.path.join(STATIC_DIR, filename))
+    f = open(npath)  # NOT OK
+
+
+@app.route("/path6")
+def safe_path_realpath():
+    # Normalized (by `realpath` that also follows symlinks), and checked properly
+    filename = request.args.get('filename', '')
+    npath = os.path.realpath(os.path.join(STATIC_DIR, filename))
+    if npath.startswith(STATIC_DIR):
+        f = open(npath)  # OK
+
+
+@app.route("/path6")
+def unsafe_path_abspath():
+    # Normalized (by `abspath`), but not checked properly
+    filename = request.args.get('filename', '')
+    npath = os.path.abspath(os.path.join(STATIC_DIR, filename))
+    f = open(npath)  # NOT OK
+
+
+@app.route("/path7")
+def safe_path_abspath():
+    # Normalized (by `abspath`), and checked properly
+    filename = request.args.get('filename', '')
+    npath = os.path.abspath(os.path.join(STATIC_DIR, filename))
+    if npath.startswith(STATIC_DIR):
+        f = open(npath)  # OK
+
+
+@app.route("/int-only/<int:foo_id>")
+def flask_int_only(foo_id):
+    # This is OK, since the flask routing ensures that `foo_id` MUST be an integer.
+    path = os.path.join(STATIC_DIR, foo_id)
+    f = open(path)  # OK TODO: FP
+
+
+@app.route("/not-path/<foo>")
+def flask_not_path(foo):
+    # On UNIX systems, this is OK, since without being marked as `<path:foo>`, flask
+    # routing ensures that `foo` cannot contain forward slashes (not by using %2F either).
+    path = os.path.join(STATIC_DIR, foo)
+    f = open(path)  # OK if only running on UNIX systems, NOT OK if could be running on windows
+
+
+@app.route("/no-dot-dot")
+def no_dot_dot():
+    filename = request.args.get('filename', '')
+    path = os.path.join(STATIC_DIR, filename)
+    # Note: even for UNIX-only programs, this check is not good enough, since it doesn't
+    # handle if `filename` is an absolute path
+    if '../' in path:
+        return "not this time"
+    f = open(path)  # NOT OK
+
+
+@app.route("/no-dot-dot-with-prefix")
+def no_dot_dot_with_prefix():
+    filename = request.args.get('filename', '')
+    path = os.path.join(STATIC_DIR, "img-"+filename)
+    # Note: Since `filename` has a prefix, it's not possible to use an absolute path.
+    # Therefore, for UNIX-only programs, the `../` check is enough to stop path injections.
+    if '../' in path:
+        return "not this time"
+    f = open(path)  # OK if only running on UNIX systems, NOT OK if could be running on windows
+
+
+@app.route("/replace-slash")
+def replace_slash():
+    filename = request.args.get('filename', '')
+    path = os.path.join(STATIC_DIR, filename)
+    sanitized = path.replace("/", "_")
+    f = open(sanitized)  # OK if only running on UNIX systems, NOT OK if could be running on windows
+
+
+@app.route("/stackoverflow-solution")
+def stackoverflow_solution():
+    # Solution provided in https://stackoverflow.com/a/45188896
+    filename = request.args.get('filename', '')
+    path = os.path.join(STATIC_DIR, filename)
+    if os.path.commonprefix((os.path.realpath(path), STATIC_DIR)) != STATIC_DIR:
+        return "not this time"
+    f = open(path) # OK TODO: FP

python/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected

@@ -1,7 +1,15 @@
 edges
-| reflected_xss.py:8:18:8:29 | ControlFlowNode for Attribute | reflected_xss.py:9:26:9:53 | ControlFlowNode for BinaryExpr |
+| reflected_xss.py:9:18:9:29 | ControlFlowNode for Attribute | reflected_xss.py:10:26:10:53 | ControlFlowNode for BinaryExpr |
+| reflected_xss.py:21:23:21:34 | ControlFlowNode for Attribute | reflected_xss.py:22:26:22:41 | ControlFlowNode for Attribute() |
+| reflected_xss.py:27:23:27:34 | ControlFlowNode for Attribute | reflected_xss.py:28:26:28:41 | ControlFlowNode for Attribute() |
 nodes
-| reflected_xss.py:8:18:8:29 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
-| reflected_xss.py:9:26:9:53 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
+| reflected_xss.py:9:18:9:29 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| reflected_xss.py:10:26:10:53 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
+| reflected_xss.py:21:23:21:34 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| reflected_xss.py:22:26:22:41 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| reflected_xss.py:27:23:27:34 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| reflected_xss.py:28:26:28:41 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
 #select
-| reflected_xss.py:9:26:9:53 | ControlFlowNode for BinaryExpr | reflected_xss.py:8:18:8:29 | ControlFlowNode for Attribute | reflected_xss.py:9:26:9:53 | ControlFlowNode for BinaryExpr | Cross-site scripting vulnerability due to $@. | reflected_xss.py:8:18:8:29 | ControlFlowNode for Attribute | a user-provided value |
+| reflected_xss.py:10:26:10:53 | ControlFlowNode for BinaryExpr | reflected_xss.py:9:18:9:29 | ControlFlowNode for Attribute | reflected_xss.py:10:26:10:53 | ControlFlowNode for BinaryExpr | Cross-site scripting vulnerability due to $@. | reflected_xss.py:9:18:9:29 | ControlFlowNode for Attribute | a user-provided value |
+| reflected_xss.py:22:26:22:41 | ControlFlowNode for Attribute() | reflected_xss.py:21:23:21:34 | ControlFlowNode for Attribute | reflected_xss.py:22:26:22:41 | ControlFlowNode for Attribute() | Cross-site scripting vulnerability due to $@. | reflected_xss.py:21:23:21:34 | ControlFlowNode for Attribute | a user-provided value |
+| reflected_xss.py:28:26:28:41 | ControlFlowNode for Attribute() | reflected_xss.py:27:23:27:34 | ControlFlowNode for Attribute | reflected_xss.py:28:26:28:41 | ControlFlowNode for Attribute() | Cross-site scripting vulnerability due to $@. | reflected_xss.py:27:23:27:34 | ControlFlowNode for Attribute | a user-provided value |

python/ql/test/query-tests/Security/CWE-079/reflected_xss.py

@@ -1,3 +1,4 @@
+import json
 from flask import Flask, request, make_response, escape
 
 app = Flask(__name__)
@@ -13,3 +14,15 @@ def unsafe():
 def safe():
     first_name = request.args.get("name", "")
     return make_response("Your name is " + escape(first_name))  # OK
+
+
+@app.route("/unsafe/json")
+def unsafe_json():
+    data = json.loads(request.data)
+    return make_response(json.dumps(data))  # NOT OK
+
+
+@app.route("/safe/json")
+def safe_json():
+    data = json.loads(request.data)
+    return make_response(json.dumps(data), 200, {'Content-Type': 'application/json'})  # OK, FP

python/ql/test/query-tests/Security/CWE-094/CodeInjection.expected

@@ -2,12 +2,16 @@ edges
 | code_injection.py:6:12:6:23 | ControlFlowNode for Attribute | code_injection.py:7:10:7:13 | ControlFlowNode for code |
 | code_injection.py:6:12:6:23 | ControlFlowNode for Attribute | code_injection.py:8:10:8:13 | ControlFlowNode for code |
 | code_injection.py:6:12:6:23 | ControlFlowNode for Attribute | code_injection.py:10:10:10:12 | ControlFlowNode for cmd |
+| code_injection.py:18:16:18:27 | ControlFlowNode for Attribute | code_injection.py:21:20:21:27 | ControlFlowNode for obj_name |
 nodes
 | code_injection.py:6:12:6:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
 | code_injection.py:7:10:7:13 | ControlFlowNode for code | semmle.label | ControlFlowNode for code |
 | code_injection.py:8:10:8:13 | ControlFlowNode for code | semmle.label | ControlFlowNode for code |
 | code_injection.py:10:10:10:12 | ControlFlowNode for cmd | semmle.label | ControlFlowNode for cmd |
+| code_injection.py:18:16:18:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| code_injection.py:21:20:21:27 | ControlFlowNode for obj_name | semmle.label | ControlFlowNode for obj_name |
 #select
 | code_injection.py:7:10:7:13 | ControlFlowNode for code | code_injection.py:6:12:6:23 | ControlFlowNode for Attribute | code_injection.py:7:10:7:13 | ControlFlowNode for code | $@ flows to here and is interpreted as code. | code_injection.py:6:12:6:23 | ControlFlowNode for Attribute | A user-provided value |
 | code_injection.py:8:10:8:13 | ControlFlowNode for code | code_injection.py:6:12:6:23 | ControlFlowNode for Attribute | code_injection.py:8:10:8:13 | ControlFlowNode for code | $@ flows to here and is interpreted as code. | code_injection.py:6:12:6:23 | ControlFlowNode for Attribute | A user-provided value |
 | code_injection.py:10:10:10:12 | ControlFlowNode for cmd | code_injection.py:6:12:6:23 | ControlFlowNode for Attribute | code_injection.py:10:10:10:12 | ControlFlowNode for cmd | $@ flows to here and is interpreted as code. | code_injection.py:6:12:6:23 | ControlFlowNode for Attribute | A user-provided value |
+| code_injection.py:21:20:21:27 | ControlFlowNode for obj_name | code_injection.py:18:16:18:27 | ControlFlowNode for Attribute | code_injection.py:21:20:21:27 | ControlFlowNode for obj_name | $@ flows to here and is interpreted as code. | code_injection.py:18:16:18:27 | ControlFlowNode for Attribute | A user-provided value |

python/ql/test/query-tests/Security/CWE-094/code_injection.py

@@ -4,7 +4,19 @@ app = Flask(__name__)
 @app.route("/code-execution")
 def code_execution():
     code = request.args.get("code")
-    exec(code)
-    eval(code)
+    exec(code) # NOT OK
+    eval(code) # NOT OK
     cmd = compile(code, "<filename>", "exec")
-    exec(cmd)
+    exec(cmd) # NOT OK
+
+
+@app.route("/safe-code-execution")
+def code_execution():
+    foo = 42
+    bar = 43
+
+    obj_name = request.args.get("obj")
+    if obj_name == "foo" or obj_name == "bar":
+        # TODO: Should not alert on this
+        obj = eval(obj_name) # OK
+        print(obj, obj*10)