Swift: Standardize the taint sources, sinks, sanitizers.

Author: geoffw0

swift/ql/lib/codeql/swift/security/CleartextStorageDatabaseExtensions.qll

@@ -4,17 +4,35 @@
  */
 
 import swift
+import codeql.swift.security.SensitiveExprs
 import codeql.swift.dataflow.DataFlow
 
 /**
- * A `DataFlow::Node` that is something stored in a local database.
+ * A dataflow sink for cleartext database storage vulnerabilities. That is,
+ * a `DataFlow::Node` that is something stored in a local database.
  */
-abstract class Stored extends DataFlow::Node { }
+abstract class CleartextStorageDatabaseSink extends DataFlow::Node { }
+
+/**
+ * A sanitizer for cleartext database storage vulnerabilities.
+ */
+abstract class CleartextStorageDatabaseSanitizer extends DataFlow::Node { }
+
+/**
+ * A unit class for adding additional taint steps.
+ */
+class CleartextStorageDatabaseAdditionalTaintStep extends Unit {
+  /**
+   * Holds if the step from `node1` to `node2` should be considered a taint
+   * step for paths related to cleartext database storage vulnerabilities.
+   */
+  abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
+}
 
 /**
  * A `DataFlow::Node` that is an expression stored with the Core Data library.
  */
-class CoreDataStore extends Stored {
+class CoreDataStore extends CleartextStorageDatabaseSink {
   CoreDataStore() {
     // values written into Core Data objects through `set*Value` methods are a sink.
     exists(CallExpr call |
@@ -42,7 +60,7 @@ class CoreDataStore extends Stored {
  * A `DataFlow::Node` that is an expression stored with the Realm database
  * library.
  */
-class RealmStore extends Stored instanceof DataFlow::PostUpdateNode {
+class RealmStore extends CleartextStorageDatabaseSink instanceof DataFlow::PostUpdateNode {
   RealmStore() {
     // any write into a class derived from `RealmSwiftObject` is a sink. For
     // example in `realmObj.data = sensitive` the post-update node corresponding
@@ -59,7 +77,7 @@ class RealmStore extends Stored instanceof DataFlow::PostUpdateNode {
 /**
  * A `DataFlow::Node` that is an expression stored with the GRDB library.
  */
-class GrdbStore extends Stored {
+class GrdbStore extends CleartextStorageDatabaseSink {
   GrdbStore() {
     exists(CallExpr call, MethodDecl method |
       call.getStaticTarget() = method and
@@ -110,3 +128,25 @@ class GrdbStore extends Stored {
     )
   }
 }
+
+/**
+ * An encryption sanitizer for cleartext database storage vulnerabilities.
+ */
+class CleartextStorageDatabaseEncryptionSanitizer extends CleartextStorageDatabaseSanitizer {
+  CleartextStorageDatabaseEncryptionSanitizer() {
+    this.asExpr() instanceof EncryptedExpr
+  }
+}
+
+/**
+ * An additional taint step for cleartext database storage vulnerabilities.
+ * Needed until we have proper content flow through arrays.
+ */
+class CleartextStorageDatabaseArrayAdditionalTaintStep extends CleartextStorageDatabaseAdditionalTaintStep {
+  override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    exists(ArrayExpr arr |
+      nodeFrom.asExpr() = arr.getAnElement() and
+      nodeTo.asExpr() = arr
+    )
+  }
+}

swift/ql/lib/codeql/swift/security/CleartextStorageDatabaseQuery.qll

@@ -18,24 +18,19 @@ class CleartextStorageConfig extends TaintTracking::Configuration {
 
   override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr }
 
-  override predicate isSink(DataFlow::Node node) { node instanceof Stored }
+  override predicate isSink(DataFlow::Node node) { node instanceof CleartextStorageDatabaseSink }
 
-  override predicate isSanitizerIn(DataFlow::Node node) {
-    // make sources barriers so that we only report the closest instance
-    isSource(node)
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer instanceof CleartextStorageDatabaseSanitizer
   }
 
-  override predicate isSanitizer(DataFlow::Node node) {
-    // encryption barrier
-    node.asExpr() instanceof EncryptedExpr
+  override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    any(CleartextStorageDatabaseAdditionalTaintStep s).step(nodeFrom, nodeTo)
   }
 
-  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-    // Needed until we have proper content flow through arrays
-    exists(ArrayExpr arr |
-      node1.asExpr() = arr.getAnElement() and
-      node2.asExpr() = arr
-    )
+  override predicate isSanitizerIn(DataFlow::Node node) {
+    // make sources barriers so that we only report the closest instance
+    isSource(node)
   }
 
   override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {

swift/ql/lib/codeql/swift/security/CleartextStoragePreferencesExtensions.qll

@@ -4,17 +4,36 @@
  */
 
 import swift
+import codeql.swift.security.SensitiveExprs
 import codeql.swift.dataflow.DataFlow
 
 /**
- * A `DataFlow::Node` of something that gets stored in an application preference store.
+ * A dataflow sink for cleartext preferences storage vulnerabilities. That is,
+ * a `DataFlow::Node` of something that gets stored in an application
+ * preference store.
  */
-abstract class Stored extends DataFlow::Node {
+abstract class CleartextStoragePreferencesSink extends DataFlow::Node {
   abstract string getStoreName();
 }
 
+/**
+ * A sanitizer for cleartext preferences storage vulnerabilities.
+ */
+abstract class CleartextStoragePreferencesSanitizer extends DataFlow::Node { }
+
+/**
+ * A unit class for adding additional taint steps.
+ */
+class CleartextStoragePreferencesAdditionalTaintStep extends Unit {
+  /**
+   * Holds if the step from `node1` to `node2` should be considered a taint
+   * step for paths related to cleartext preferences storage vulnerabilities.
+   */
+  abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
+}
+
 /** The `DataFlow::Node` of an expression that gets written to the user defaults database */
-class UserDefaultsStore extends Stored {
+class UserDefaultsStore extends CleartextStoragePreferencesSink {
   UserDefaultsStore() {
     exists(CallExpr call |
       call.getStaticTarget().(MethodDecl).hasQualifiedName("UserDefaults", "set(_:forKey:)") and
@@ -26,7 +45,7 @@ class UserDefaultsStore extends Stored {
 }
 
 /** The `DataFlow::Node` of an expression that gets written to the iCloud-backed NSUbiquitousKeyValueStore */
-class NSUbiquitousKeyValueStore extends Stored {
+class NSUbiquitousKeyValueStore extends CleartextStoragePreferencesSink {
   NSUbiquitousKeyValueStore() {
     exists(CallExpr call |
       call.getStaticTarget()
@@ -45,8 +64,17 @@ class NSUbiquitousKeyValueStore extends Stored {
  * object via reflection (`perform(Selector)`) or the `NSKeyValueCoding`,
  * `NSKeyValueBindingCreation` APIs. (TODO)
  */
-class NSUserDefaultsControllerStore extends Stored {
+class NSUserDefaultsControllerStore extends CleartextStoragePreferencesSink {
   NSUserDefaultsControllerStore() { none() }
 
   override string getStoreName() { result = "the user defaults database" }
 }
+
+/**
+ * An encryption sanitizer for cleartext preferences storage vulnerabilities.
+ */
+class CleartextStoragePreferencesEncryptionSanitizer extends CleartextStoragePreferencesSanitizer {
+  CleartextStoragePreferencesEncryptionSanitizer() {
+    this.asExpr() instanceof EncryptedExpr
+  }
+}

swift/ql/lib/codeql/swift/security/CleartextStoragePreferencesQuery.qll

@@ -18,15 +18,18 @@ class CleartextStorageConfig extends TaintTracking::Configuration {
 
   override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr }
 
-  override predicate isSink(DataFlow::Node node) { node instanceof Stored }
+  override predicate isSink(DataFlow::Node node) { node instanceof CleartextStoragePreferencesSink }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer instanceof CleartextStoragePreferencesSanitizer
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    any(CleartextStoragePreferencesAdditionalTaintStep s).step(nodeFrom, nodeTo)
+  }
 
   override predicate isSanitizerIn(DataFlow::Node node) {
     // make sources barriers so that we only report the closest instance
     this.isSource(node)
   }
-
-  override predicate isSanitizer(DataFlow::Node node) {
-    // encryption barrier
-    node.asExpr() instanceof EncryptedExpr
-  }
 }

swift/ql/lib/codeql/swift/security/CleartextTransmissionExtensions.qll

@@ -4,24 +4,42 @@
  */
 
 import swift
+import codeql.swift.security.SensitiveExprs
 import codeql.swift.dataflow.DataFlow
 
 /**
- * An `Expr` that is transmitted over a network.
+ * A dataflow sink for cleartext transmission vulnerabilities. That is,
+ * a `DataFlow::Node` of something that is transmitted over a network.
  */
-abstract class Transmitted extends Expr { }
+abstract class CleartextTransmissionSink extends DataFlow::Node { }
+
+/**
+ * A sanitizer for cleartext transmission vulnerabilities.
+ */
+abstract class CleartextTransmissionSanitizer extends DataFlow::Node { }
+
+/**
+ * A unit class for adding additional taint steps.
+ */
+class CleartextTransmissionAdditionalTaintStep extends Unit {
+  /**
+   * Holds if the step from `node1` to `node2` should be considered a taint
+   * step for paths related to cleartext transmission vulnerabilities.
+   */
+  abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
+}
 
 /**
  * An `Expr` that is transmitted with `NWConnection.send`.
  */
-class NWConnectionSend extends Transmitted {
+class NWConnectionSend extends CleartextTransmissionSink {
   NWConnectionSend() {
     // `content` arg to `NWConnection.send` is a sink
     exists(CallExpr call |
       call.getStaticTarget()
           .(MethodDecl)
           .hasQualifiedName("NWConnection", "send(content:contentContext:isComplete:completion:)") and
-      call.getArgument(0).getExpr() = this
+      call.getArgument(0).getExpr() = this.asExpr()
     )
   }
 }
@@ -30,33 +48,42 @@ class NWConnectionSend extends Transmitted {
  * An `Expr` that is used to form a `URL`. Such expressions are very likely to
  * be transmitted over a network, because that's what URLs are for.
  */
-class Url extends Transmitted {
+class Url extends CleartextTransmissionSink {
   Url() {
     // `string` arg in `URL.init` is a sink
     // (we assume here that the URL goes on to be used in a network operation)
     exists(CallExpr call |
       call.getStaticTarget()
           .(MethodDecl)
           .hasQualifiedName("URL", ["init(string:)", "init(string:relativeTo:)"]) and
-      call.getArgument(0).getExpr() = this
+      call.getArgument(0).getExpr() = this.asExpr()
     )
   }
 }
 
 /**
  * An `Expr` that transmitted through the Alamofire library.
  */
-class AlamofireTransmitted extends Transmitted {
+class AlamofireTransmitted extends CleartextTransmissionSink {
   AlamofireTransmitted() {
     // sinks are the first argument containing the URL, and the `parameters`
     // and `headers` arguments to appropriate methods of `Session`.
     exists(CallExpr call, string fName |
       call.getStaticTarget().(MethodDecl).hasQualifiedName("Session", fName) and
       fName.regexpMatch("(request|streamRequest|download)\\(.*") and
       (
-        call.getArgument(0).getExpr() = this or
-        call.getArgumentWithLabel(["headers", "parameters"]).getExpr() = this
+        call.getArgument(0).getExpr() = this.asExpr() or
+        call.getArgumentWithLabel(["headers", "parameters"]).getExpr() = this.asExpr()
       )
     )
   }
 }
+
+/**
+ * An encryption sanitizer for cleartext transmission vulnerabilities.
+ */
+class CleartextTransmissionEncryptionSanitizer extends CleartextTransmissionSanitizer {
+  CleartextTransmissionEncryptionSanitizer() {
+    this.asExpr() instanceof EncryptedExpr
+  }
+}

swift/ql/lib/codeql/swift/security/CleartextTransmissionQuery.qll

@@ -18,15 +18,18 @@ class CleartextTransmissionConfig extends TaintTracking::Configuration {
 
   override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr }
 
-  override predicate isSink(DataFlow::Node node) { node.asExpr() instanceof Transmitted }
+  override predicate isSink(DataFlow::Node node) { node instanceof CleartextTransmissionSink }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer instanceof CleartextTransmissionSanitizer
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    any(CleartextTransmissionAdditionalTaintStep s).step(nodeFrom, nodeTo)
+  }
 
   override predicate isSanitizerIn(DataFlow::Node node) {
     // make sources barriers so that we only report the closest instance
     isSource(node)
   }
-
-  override predicate isSanitizer(DataFlow::Node node) {
-    // encryption barrier
-    node.asExpr() instanceof EncryptedExpr
-  }
 }

swift/ql/src/queries/Security/CWE-312/CleartextStoragePreferences.ql

@@ -29,6 +29,6 @@ from CleartextStorageConfig config, DataFlow::PathNode sourceNode, DataFlow::Pat
 where config.hasFlowPath(sourceNode, sinkNode)
 select cleanupNode(sinkNode.getNode()), sourceNode, sinkNode,
   "This operation stores '" + sinkNode.getNode().toString() + "' in " +
-    sinkNode.getNode().(Stored).getStoreName() +
+    sinkNode.getNode().(CleartextStoragePreferencesSink).getStoreName() +
     ". It may contain unencrypted sensitive data from $@.", sourceNode,
   sourceNode.getNode().toString()