Swift: Define barriers, additional flow steps and sinks.

Author: geoffw0

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

@@ -4,33 +4,61 @@
  */
 
 import swift
-import codeql.swift.security.SensitiveExprs
 import codeql.swift.dataflow.DataFlow
-import codeql.swift.dataflow.TaintTracking
+import codeql.swift.dataflow.ExternalFlow
+private import codeql.swift.security.WeakSensitiveDataHashingExtensions
 
-class WeakPasswordHashingSink extends DataFlow::Node {
+/**
+ * A dataflow sink for weak password hashing vulnerabilities. That is,
+ * a `DataFlow::Node` that is passed into a weak password hashing function.
+ */
+abstract class WeakPasswordHashingSink extends DataFlow::Node {
+  /**
+   * Gets the name of the hashing algorithm, for display.
+   */
+  abstract string getAlgorithm();
+}
+
+/**
+ * A barrier for weak password hashing vulnerabilities.
+ */
+abstract class WeakPasswordHashingBarrier extends DataFlow::Node { }
+
+/**
+ * A unit class for adding additional flow steps.
+ */
+class WeakPasswordHashingAdditionalFlowStep extends Unit {
+  /**
+   * Holds if the step from `node1` to `node2` should be considered a flow
+   * step for paths related to weak password hashing vulnerabilities.
+   */
+  abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
+}
+
+/**
+ * A sink inherited from weak sensitive data hashing. Password hashing has
+ * stronger requirements than sensitive data hashing, since (in addition to
+ * its particular qualities) a password *is* sensitive data. Thus, any sink
+ * for the weak sensitive data hashing query is a sink for weak password
+ * hashing as well.
+ */
+private class InheritedWeakPasswordHashingSink extends WeakPasswordHashingSink {
+ InheritedWeakPasswordHashingSink() {
+   this instanceof WeakSensitiveDataHashingSink
+ }
+
+ override string getAlgorithm() { result = this.(WeakSensitiveDataHashingSink).getAlgorithm() }
+}
+
+/**
+ * A sink defined in a CSV model.
+ */
+private class DefaultWeakPasswordHashingSink extends WeakPasswordHashingSink {
   string algorithm;
 
-  WeakPasswordHashingSink() {
-    // a call to System.Security.Cryptography.MD5/SHA*.ComputeHash/ComputeHashAsync/HashData/HashDataAsync
-    exists(MethodCall call, string name |
-      (
-        call.getTarget().getName() = name
-        and name in ["ComputeHash", "ComputeHashAsync", "HashData", "HashDataAsync"]
-      )
-      // with this as the first argument - not arg 0, since arg 0 is 'this' for methods
-      and call.getArgument(0) = this.asExpr()
-      and
-      // the call is to a method in the System.Security.Cryptography.MD* class
-      // or the System.Security.Cryptography.SHA* classes
-      (
-        call.getQualifier().getType().getName() = algorithm
-        and algorithm.matches(["MD%","SHA%"])
-      )
-    )
+  DefaultWeakPasswordHashingSink() {
+    sinkNode(this, "weak-password-hash-input-" + algorithm)
   }
 
-  string getAlgorithm() {
-    result = algorithm
-  }
+  override string getAlgorithm() { result = algorithm }
 }

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

@@ -18,6 +18,8 @@ module WeakHashingPasswordConfig implements DataFlow::ConfigSig {
 
   predicate isSink(DataFlow::Node node) { node instanceof WeakPasswordHashingSink }
 
+  predicate isBarrier(DataFlow::Node node) { node instanceof WeakPasswordHashingBarrier }
+
   predicate isBarrierIn(DataFlow::Node node) {
     // make sources barriers so that we only report the closest instance
     isSource(node)
@@ -27,6 +29,10 @@ module WeakHashingPasswordConfig implements DataFlow::ConfigSig {
     // make sinks barriers so that we only report the closest instance
     isSink(node)
   }
+
+  predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    any(WeakPasswordHashingAdditionalFlowStep s).step(nodeFrom, nodeTo)
+  }
 }
 
 module WeakHashingFlow = TaintTracking::Global<WeakHashingPasswordConfig>;