Python: Allow models-as-data sanitizers

python/ql/lib/semmle/python/Concepts.qll

@@ -116,6 +116,16 @@ module SystemCommandExecution {
 class FileSystemAccess extends DataFlow::Node instanceof FileSystemAccess::Range {
   /** Gets an argument to this file system access that is interpreted as a path. */
   DataFlow::Node getAPathArgument() { result = super.getAPathArgument() }
+
+  /**
+   * Gets an argument to this file system access that is interpreted as a path
+   * which is vulnerable to path injection.
+   *
+   * By default all path arguments are considered vulnerable, but this can be overridden to
+   * exclude certain arguments that are known to be safe, for example because they are
+   * restricted to a specific directory.
+   */
+  DataFlow::Node getAVulnerablePathArgument() { result = super.getAVulnerablePathArgument() }
 }
 
 /** Provides a class for modeling new file system access APIs. */
@@ -130,6 +140,16 @@ module FileSystemAccess {
   abstract class Range extends DataFlow::Node {
     /** Gets an argument to this file system access that is interpreted as a path. */
     abstract DataFlow::Node getAPathArgument();
+
+    /**
+     * Gets an argument to this file system access that is interpreted as a path
+     * which is vulnerable to path injection.
+     *
+     * By default all path arguments are considered vulnerable, but this can be overridden to
+     * exclude certain arguments that are known to be safe, for example because they are
+     * restricted to a specific directory.
+     */
+    DataFlow::Node getAVulnerablePathArgument() { result = this.getAPathArgument() }
   }
 }
 

python/ql/lib/semmle/python/frameworks/Flask.qll

@@ -621,24 +621,15 @@ module Flask {
     }
 
     override DataFlow::Node getAPathArgument() {
-      result in [
-          this.getArg(0), this.getArgByName("directory"),
-          // as described in the docs, the `filename` argument is restrained to be within
-          // the provided directory, so is not exposed to path-injection. (but is still a
-          // path-argument).
-          this.getArg(1), this.getArgByName("filename")
-        ]
+      result = this.getArg([0, 1]) or
+      result = this.getArgByName(["directory", "filename"])
     }
-  }
 
-  /**
-   * To exclude `filename` argument to `flask.send_from_directory` as a path-injection sink.
-   */
-  private class FlaskSendFromDirectoryCallFilenameSanitizer extends PathInjection::Sanitizer {
-    FlaskSendFromDirectoryCallFilenameSanitizer() {
-      this = any(FlaskSendFromDirectoryCall c).getArg(1)
-      or
-      this = any(FlaskSendFromDirectoryCall c).getArgByName("filename")
+    override DataFlow::Node getAVulnerablePathArgument() {
+      result = this.getAPathArgument() and
+      // as described in the docs, the `filename` argument is restricted to be within
+      // the provided directory, so is not exposed to path-injection.
+      not result in [this.getArg(1), this.getArgByName("filename")]
     }
   }
 

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

@@ -60,4 +60,11 @@ module CodeInjection {
 
   /** DEPRECATED: Use ConstCompareAsSanitizerGuard instead. */
   deprecated class StringConstCompareAsSanitizerGuard = ConstCompareAsSanitizerGuard;
+
+  /**
+   * A sanitizer defined via models-as-data with kind "code-injection".
+   */
+  class SanitizerFromModel extends Sanitizer {
+    SanitizerFromModel() { ModelOutput::barrierNode(this, "code-injection") }
+  }
 }

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

@@ -95,4 +95,11 @@ module CommandInjection {
 
   /** DEPRECATED: Use ConstCompareAsSanitizerGuard instead. */
   deprecated class StringConstCompareAsSanitizerGuard = ConstCompareAsSanitizerGuard;
+
+  /**
+   * A sanitizer defined via models-as-data with kind "command-injection".
+   */
+  class SanitizerFromModel extends Sanitizer {
+    SanitizerFromModel() { ModelOutput::barrierNode(this, "command-injection") }
+  }
 }

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

@@ -106,4 +106,11 @@ module LogInjection {
       this.getArg(0).asExpr().(StringLiteral).getText() in ["\r\n", "\n"]
     }
   }
+
+  /**
+   * A sanitizer defined via models-as-data with kind "log-injection".
+   */
+  class SanitizerFromModel extends Sanitizer {
+    SanitizerFromModel() { ModelOutput::barrierNode(this, "log-injection") }
+  }
 }

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

@@ -57,7 +57,7 @@ module PathInjection {
    */
   class FileSystemAccessAsSink extends Sink {
     FileSystemAccessAsSink() {
-      this = any(FileSystemAccess e).getAPathArgument() and
+      this = any(FileSystemAccess e).getAVulnerablePathArgument() and
       // since implementation of Path.open in pathlib.py is like
       // ```py
       // def open(self, ...):
@@ -98,4 +98,11 @@ module PathInjection {
 
   /** DEPRECATED: Use ConstCompareAsSanitizerGuard instead. */
   deprecated class StringConstCompareAsSanitizerGuard = ConstCompareAsSanitizerGuard;
+
+  /**
+   * A sanitizer defined via models-as-data with kind "path-injection".
+   */
+  class SanitizerFromModel extends Sanitizer {
+    SanitizerFromModel() { ModelOutput::barrierNode(this, "path-injection") }
+  }
 }

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

@@ -84,4 +84,11 @@ module ReflectedXss {
 
   /** DEPRECATED: Use ConstCompareAsSanitizerGuard instead. */
   deprecated class StringConstCompareAsSanitizerGuard = ConstCompareAsSanitizerGuard;
+
+  /**
+   * A sanitizer defined via models-as-data with kind "html-injection" or "js-injection".
+   */
+  class SanitizerFromModel extends Sanitizer {
+    SanitizerFromModel() { ModelOutput::barrierNode(this, ["html-injection", "js-injection"]) }
+  }
 }

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

@@ -69,4 +69,11 @@ module SqlInjection {
   private class DataAsSqlSink extends Sink {
     DataAsSqlSink() { ModelOutput::sinkNode(this, "sql-injection") }
   }
+
+  /**
+   * A sanitizer defined via models-as-data with kind "sql-injection".
+   */
+  class SanitizerFromModel extends Sanitizer {
+    SanitizerFromModel() { ModelOutput::barrierNode(this, "sql-injection") }
+  }
 }

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

@@ -65,4 +65,11 @@ module UnsafeDeserialization {
 
   /** DEPRECATED: Use ConstCompareAsSanitizerGuard instead. */
   deprecated class StringConstCompareAsSanitizerGuard = ConstCompareAsSanitizerGuard;
+
+  /**
+   * A sanitizer defined via models-as-data with kind "unsafe-deserialization".
+   */
+  class SanitizerFromModel extends Sanitizer {
+    SanitizerFromModel() { ModelOutput::barrierNode(this, "unsafe-deserialization") }
+  }
 }

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

@@ -162,7 +162,8 @@ module UrlRedirect {
   deprecated class StringConstCompareAsSanitizerGuard = ConstCompareAsSanitizerGuard;
 
   /**
-   * A sanitizer defined via models-as-data with kind "url-redirection".
+   * A sanitizer which sanitizes all flow states, defined via models-as-data
+   * with kind "url-redirection".
    */
   class SanitizerFromModel extends Sanitizer {
     SanitizerFromModel() { ModelOutput::barrierNode(this, "url-redirection") }

python/ql/lib/utils/test/PrettyPrintModels.ql

@@ -0,0 +1,6 @@
+/**
+ * @kind test-postprocess
+ */
+
+import semmle.python.frameworks.data.internal.ApiGraphModels
+import codeql.dataflow.test.ProvenancePathGraph::TestPostProcessing::TranslateProvenanceResults<interpretModelForTest/2>

python/ql/src/Security/CWE-798/HardcodedCredentials.ql

@@ -105,6 +105,15 @@ class CredentialSink extends DataFlow::Node {
   }
 }
 
+class CredentialSanitizer extends DataFlow::Node {
+  CredentialSanitizer() {
+    exists(string s | s.matches("credentials-%") |
+      // Whatever the string, this will sanitize flow to all credential sinks.
+      ModelOutput::barrierNode(this, s)
+    )
+  }
+}
+
 /**
  * Gets a regular expression for matching names of locations (variables, parameters, keys) that
  * indicate the value being held is a credential.
@@ -120,6 +129,8 @@ private module HardcodedCredentialsConfig implements DataFlow::ConfigSig {
 
   predicate isSink(DataFlow::Node sink) { sink instanceof CredentialSink }
 
+  predicate isBarrier(DataFlow::Node node) { node instanceof CredentialSanitizer }
+
   predicate observeDiffInformedIncrementalMode() { any() }
 }
 

python/ql/test/experimental/query-tests/Security/CWE-022-UnsafeUnpacking/UnsafeUnpack.expected

@@ -1,3 +1,17 @@
+#select
+| UnsafeUnpack.py:19:35:19:41 | ControlFlowNode for tarpath | UnsafeUnpack.py:5:26:5:32 | ControlFlowNode for ImportMember | UnsafeUnpack.py:19:35:19:41 | ControlFlowNode for tarpath | Unsafe extraction from a malicious tarball retrieved from a remote location. |
+| UnsafeUnpack.py:34:23:34:38 | ControlFlowNode for local_ziped_path | UnsafeUnpack.py:33:50:33:65 | ControlFlowNode for local_ziped_path | UnsafeUnpack.py:34:23:34:38 | ControlFlowNode for local_ziped_path | Unsafe extraction from a malicious tarball retrieved from a remote location. |
+| UnsafeUnpack.py:48:23:48:37 | ControlFlowNode for compressed_file | UnsafeUnpack.py:47:20:47:34 | ControlFlowNode for compressed_file | UnsafeUnpack.py:48:23:48:37 | ControlFlowNode for compressed_file | Unsafe extraction from a malicious tarball retrieved from a remote location. |
+| UnsafeUnpack.py:52:23:52:37 | ControlFlowNode for compressed_file | UnsafeUnpack.py:51:19:51:36 | ControlFlowNode for Attribute() | UnsafeUnpack.py:52:23:52:37 | ControlFlowNode for compressed_file | Unsafe extraction from a malicious tarball retrieved from a remote location. |
+| UnsafeUnpack.py:66:23:66:37 | ControlFlowNode for compressed_file | UnsafeUnpack.py:65:19:65:31 | ControlFlowNode for Attribute | UnsafeUnpack.py:66:23:66:37 | ControlFlowNode for compressed_file | Unsafe extraction from a malicious tarball retrieved from a remote location. |
+| UnsafeUnpack.py:87:23:87:29 | ControlFlowNode for tarpath | UnsafeUnpack.py:79:16:79:28 | ControlFlowNode for Attribute | UnsafeUnpack.py:87:23:87:29 | ControlFlowNode for tarpath | Unsafe extraction from a malicious tarball retrieved from a remote location. |
+| UnsafeUnpack.py:105:35:105:42 | ControlFlowNode for savepath | UnsafeUnpack.py:103:32:103:44 | ControlFlowNode for Attribute | UnsafeUnpack.py:105:35:105:42 | ControlFlowNode for savepath | Unsafe extraction from a malicious tarball retrieved from a remote location. |
+| UnsafeUnpack.py:112:35:112:43 | ControlFlowNode for file_path | UnsafeUnpack.py:108:22:108:34 | ControlFlowNode for Attribute | UnsafeUnpack.py:112:35:112:43 | ControlFlowNode for file_path | Unsafe extraction from a malicious tarball retrieved from a remote location. |
+| UnsafeUnpack.py:120:41:120:58 | ControlFlowNode for uploaded_file_path | UnsafeUnpack.py:116:27:116:39 | ControlFlowNode for Attribute | UnsafeUnpack.py:120:41:120:58 | ControlFlowNode for uploaded_file_path | Unsafe extraction from a malicious tarball retrieved from a remote location. |
+| UnsafeUnpack.py:142:49:142:51 | ControlFlowNode for tar | UnsafeUnpack.py:140:23:140:35 | ControlFlowNode for Attribute | UnsafeUnpack.py:142:49:142:51 | ControlFlowNode for tar | Unsafe extraction from a malicious tarball retrieved from a remote location. |
+| UnsafeUnpack.py:167:67:167:72 | ControlFlowNode for result | UnsafeUnpack.py:158:32:158:44 | ControlFlowNode for Attribute | UnsafeUnpack.py:167:67:167:72 | ControlFlowNode for result | Unsafe extraction from a malicious tarball retrieved from a remote location. |
+| UnsafeUnpack.py:176:1:176:34 | ControlFlowNode for Attribute() | UnsafeUnpack.py:79:16:79:28 | ControlFlowNode for Attribute | UnsafeUnpack.py:176:1:176:34 | ControlFlowNode for Attribute() | Unsafe extraction from a malicious tarball retrieved from a remote location. |
+| UnsafeUnpack.py:201:29:201:36 | ControlFlowNode for Attribute | UnsafeUnpack.py:194:53:194:55 | ControlFlowNode for tmp | UnsafeUnpack.py:201:29:201:36 | ControlFlowNode for Attribute | Unsafe extraction from a malicious tarball retrieved from a remote location. |
 edges
 | UnsafeUnpack.py:5:26:5:32 | ControlFlowNode for ImportMember | UnsafeUnpack.py:5:26:5:32 | ControlFlowNode for request | provenance |  |
 | UnsafeUnpack.py:5:26:5:32 | ControlFlowNode for request | UnsafeUnpack.py:11:18:11:24 | ControlFlowNode for request | provenance |  |
@@ -75,7 +89,7 @@ edges
 | UnsafeUnpack.py:161:19:161:21 | ControlFlowNode for tar | UnsafeUnpack.py:163:33:163:35 | ControlFlowNode for tar | provenance |  |
 | UnsafeUnpack.py:161:25:161:46 | ControlFlowNode for Attribute() | UnsafeUnpack.py:161:19:161:21 | ControlFlowNode for tar | provenance |  |
 | UnsafeUnpack.py:161:38:161:45 | ControlFlowNode for savepath | UnsafeUnpack.py:161:25:161:46 | ControlFlowNode for Attribute() | provenance | Config |
-| UnsafeUnpack.py:161:38:161:45 | ControlFlowNode for savepath | UnsafeUnpack.py:161:25:161:46 | ControlFlowNode for Attribute() | provenance | MaD:70 |
+| UnsafeUnpack.py:161:38:161:45 | ControlFlowNode for savepath | UnsafeUnpack.py:161:25:161:46 | ControlFlowNode for Attribute() | provenance | MaD:1 |
 | UnsafeUnpack.py:163:23:163:28 | ControlFlowNode for member | UnsafeUnpack.py:166:37:166:42 | ControlFlowNode for member | provenance |  |
 | UnsafeUnpack.py:163:33:163:35 | ControlFlowNode for tar | UnsafeUnpack.py:163:23:163:28 | ControlFlowNode for member | provenance |  |
 | UnsafeUnpack.py:166:23:166:28 | [post] ControlFlowNode for result | UnsafeUnpack.py:167:67:167:72 | ControlFlowNode for result | provenance |  |
@@ -92,6 +106,8 @@ edges
 | UnsafeUnpack.py:176:17:176:23 | ControlFlowNode for tarpath | UnsafeUnpack.py:176:1:176:34 | ControlFlowNode for Attribute() | provenance | Config |
 | UnsafeUnpack.py:194:53:194:55 | ControlFlowNode for tmp | UnsafeUnpack.py:201:29:201:31 | ControlFlowNode for tmp | provenance |  |
 | UnsafeUnpack.py:201:29:201:31 | ControlFlowNode for tmp | UnsafeUnpack.py:201:29:201:36 | ControlFlowNode for Attribute | provenance | Config |
+models
+| 1 | Summary: tarfile; Member[open]; Argument[0,name:,2,fileobj:]; ReturnValue; taint |
 nodes
 | UnsafeUnpack.py:5:26:5:32 | ControlFlowNode for ImportMember | semmle.label | ControlFlowNode for ImportMember |
 | UnsafeUnpack.py:5:26:5:32 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
@@ -188,17 +204,3 @@ nodes
 | UnsafeUnpack.py:201:29:201:31 | ControlFlowNode for tmp | semmle.label | ControlFlowNode for tmp |
 | UnsafeUnpack.py:201:29:201:36 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
 subpaths
-#select
-| UnsafeUnpack.py:19:35:19:41 | ControlFlowNode for tarpath | UnsafeUnpack.py:5:26:5:32 | ControlFlowNode for ImportMember | UnsafeUnpack.py:19:35:19:41 | ControlFlowNode for tarpath | Unsafe extraction from a malicious tarball retrieved from a remote location. |
-| UnsafeUnpack.py:34:23:34:38 | ControlFlowNode for local_ziped_path | UnsafeUnpack.py:33:50:33:65 | ControlFlowNode for local_ziped_path | UnsafeUnpack.py:34:23:34:38 | ControlFlowNode for local_ziped_path | Unsafe extraction from a malicious tarball retrieved from a remote location. |
-| UnsafeUnpack.py:48:23:48:37 | ControlFlowNode for compressed_file | UnsafeUnpack.py:47:20:47:34 | ControlFlowNode for compressed_file | UnsafeUnpack.py:48:23:48:37 | ControlFlowNode for compressed_file | Unsafe extraction from a malicious tarball retrieved from a remote location. |
-| UnsafeUnpack.py:52:23:52:37 | ControlFlowNode for compressed_file | UnsafeUnpack.py:51:19:51:36 | ControlFlowNode for Attribute() | UnsafeUnpack.py:52:23:52:37 | ControlFlowNode for compressed_file | Unsafe extraction from a malicious tarball retrieved from a remote location. |
-| UnsafeUnpack.py:66:23:66:37 | ControlFlowNode for compressed_file | UnsafeUnpack.py:65:19:65:31 | ControlFlowNode for Attribute | UnsafeUnpack.py:66:23:66:37 | ControlFlowNode for compressed_file | Unsafe extraction from a malicious tarball retrieved from a remote location. |
-| UnsafeUnpack.py:87:23:87:29 | ControlFlowNode for tarpath | UnsafeUnpack.py:79:16:79:28 | ControlFlowNode for Attribute | UnsafeUnpack.py:87:23:87:29 | ControlFlowNode for tarpath | Unsafe extraction from a malicious tarball retrieved from a remote location. |
-| UnsafeUnpack.py:105:35:105:42 | ControlFlowNode for savepath | UnsafeUnpack.py:103:32:103:44 | ControlFlowNode for Attribute | UnsafeUnpack.py:105:35:105:42 | ControlFlowNode for savepath | Unsafe extraction from a malicious tarball retrieved from a remote location. |
-| UnsafeUnpack.py:112:35:112:43 | ControlFlowNode for file_path | UnsafeUnpack.py:108:22:108:34 | ControlFlowNode for Attribute | UnsafeUnpack.py:112:35:112:43 | ControlFlowNode for file_path | Unsafe extraction from a malicious tarball retrieved from a remote location. |
-| UnsafeUnpack.py:120:41:120:58 | ControlFlowNode for uploaded_file_path | UnsafeUnpack.py:116:27:116:39 | ControlFlowNode for Attribute | UnsafeUnpack.py:120:41:120:58 | ControlFlowNode for uploaded_file_path | Unsafe extraction from a malicious tarball retrieved from a remote location. |
-| UnsafeUnpack.py:142:49:142:51 | ControlFlowNode for tar | UnsafeUnpack.py:140:23:140:35 | ControlFlowNode for Attribute | UnsafeUnpack.py:142:49:142:51 | ControlFlowNode for tar | Unsafe extraction from a malicious tarball retrieved from a remote location. |
-| UnsafeUnpack.py:167:67:167:72 | ControlFlowNode for result | UnsafeUnpack.py:158:32:158:44 | ControlFlowNode for Attribute | UnsafeUnpack.py:167:67:167:72 | ControlFlowNode for result | Unsafe extraction from a malicious tarball retrieved from a remote location. |
-| UnsafeUnpack.py:176:1:176:34 | ControlFlowNode for Attribute() | UnsafeUnpack.py:79:16:79:28 | ControlFlowNode for Attribute | UnsafeUnpack.py:176:1:176:34 | ControlFlowNode for Attribute() | Unsafe extraction from a malicious tarball retrieved from a remote location. |
-| UnsafeUnpack.py:201:29:201:36 | ControlFlowNode for Attribute | UnsafeUnpack.py:194:53:194:55 | ControlFlowNode for tmp | UnsafeUnpack.py:201:29:201:36 | ControlFlowNode for Attribute | Unsafe extraction from a malicious tarball retrieved from a remote location. |

python/ql/test/experimental/query-tests/Security/CWE-022-UnsafeUnpacking/UnsafeUnpack.qlref

@@ -1 +1,2 @@
-experimental/Security/CWE-022bis/UnsafeUnpack.ql
+query: experimental/Security/CWE-022bis/UnsafeUnpack.ql
+postprocess: utils/test/PrettyPrintModels.ql