Swift: Query for Use of an inappropriate cryptographic hashing algorithm on passwords

swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Security.qll

@@ -8,6 +8,6 @@ private import codeql.swift.dataflow.ExternalFlow
 
 private class SensitiveSources extends SourceModelCsv {
   override predicate row(string row) {
-    row = ";;false;SecKeyCopyExternalRepresentation(_:_:);;;ReturnValue;sensitive-credential"
+    row = ";;false;SecKeyCopyExternalRepresentation(_:_:);;;ReturnValue;sensitive-password"
   }
 }

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

@@ -10,6 +10,7 @@ private import codeql.swift.dataflow.DataFlow
 private import codeql.swift.dataflow.ExternalFlow
 
 private newtype TSensitiveDataType =
+  TPassword() or
   TCredential() or
   TPrivateInfo()
 
@@ -26,18 +27,32 @@ abstract class SensitiveDataType extends TSensitiveDataType {
 }
 
 /**
- * The type of sensitive expression for passwords and other credentials.
+ * The type of sensitive expression for passwords.
+ */
+class SensitivePassword extends SensitiveDataType, TPassword {
+  override string toString() { result = "password" }
+
+  override string getRegexp() {
+    result = HeuristicNames::maybeSensitiveRegexp(SensitiveDataClassification::password())
+    or
+    result = "(?is).*pass.?phrase.*"
+  }
+}
+
+/**
+ * The type of sensitive expression for credentials and secrets other than passwords.
  */
 class SensitiveCredential extends SensitiveDataType, TCredential {
   override string toString() { result = "credential" }
 
   override string getRegexp() {
     exists(SensitiveDataClassification classification |
+      not classification = SensitiveDataClassification::password() and // covered by `SensitivePassword`
       not classification = SensitiveDataClassification::id() and // not accurate enough
       result = HeuristicNames::maybeSensitiveRegexp(classification)
     )
     or
-    result = "(?is).*((account|accnt|licen(se|ce)).?(id|key)|one.?time.?code|pass.?phrase).*"
+    result = "(?is).*((account|accnt|licen(se|ce)).?(id|key)|one.?time.?code).*"
   }
 }
 
@@ -57,7 +72,8 @@ class SensitivePrivateInfo extends SensitiveDataType, TPrivateInfo {
         // Contact information, such as home addresses
         "post.?code|zip.?code|home.?addr|" +
         // and telephone numbers
-        "(mob(ile)?|home).?(num|no|tel|phone)|(tel|fax).?(num|no|phone)|" + "emergency.?contact|" +
+        "(mob(ile)?|home).?(num|no|tel|phone)|(tel|fax|phone).?(num|no)|telephone|" +
+        "emergency.?contact|" +
         // Geographic location - where the user is (or was)
         "l(atitude|ongitude)|nationality|" +
         // Financial data - such as credit card numbers, salary, bank accounts, and debts
@@ -176,6 +192,11 @@ class SensitiveExpr extends Expr {
     not label.regexpMatch(regexpProbablySafe())
     or
     (
+      // modeled sensitive password
+      sourceNode(DataFlow::exprNode(this), "sensitive-password") and
+      sensitiveType = TPassword() and
+      label = "password"
+      or
       // modeled sensitive credential
       sourceNode(DataFlow::exprNode(this), "sensitive-credential") and
       sensitiveType = TCredential() and

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

@@ -0,0 +1,128 @@
+/**
+ * Provides classes and predicates for reasoning about use of inappropriate
+ * cryptographic hashing algorithms on passwords.
+ */
+
+import swift
+import codeql.swift.dataflow.DataFlow
+import codeql.swift.dataflow.ExternalFlow
+private import codeql.swift.security.WeakSensitiveDataHashingExtensions
+
+/**
+ * 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() }
+}
+
+private class WeakSensitiveDataHashingSinks extends SinkModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        // CryptoKit
+        // (SHA-256, SHA-384 and SHA-512 are all variants of the SHA-2 algorithm)
+        ";SHA256;true;hash(data:);;;Argument[0];weak-password-hash-input-SHA256",
+        ";SHA256;true;update(data:);;;Argument[0];weak-password-hash-input-SHA256",
+        ";SHA256;true;update(bufferPointer:);;;Argument[0];weak-password-hash-input-SHA256",
+        ";SHA384;true;hash(data:);;;Argument[0];weak-password-hash-input-SHA384",
+        ";SHA384;true;update(data:);;;Argument[0];weak-password-hash-input-SHA384",
+        ";SHA384;true;update(bufferPointer:);;;Argument[0];weak-password-hash-input-SHA384",
+        ";SHA512;true;hash(data:);;;Argument[0];weak-password-hash-input-SHA512",
+        ";SHA512;true;update(data:);;;Argument[0];weak-password-hash-input-SHA512",
+        ";SHA512;true;update(bufferPointer:);;;Argument[0];weak-password-hash-input-SHA512",
+        // CryptoSwift
+        ";SHA2;true;calculate(for:);;;Argument[0];weak-password-hash-input-SHA2",
+        ";SHA2;true;callAsFunction(_:);;;Argument[0];weak-password-hash-input-SHA2",
+        ";SHA2;true;process64(block:currentHash:);;;Argument[0];weak-password-hash-input-SHA2",
+        ";SHA2;true;process32(block:currentHash:);;;Argument[0];weak-password-hash-input-SHA2",
+        ";SHA2;true;update(withBytes:isLast:);;;Argument[0];weak-password-hash-input-SHA2",
+        ";SHA3;true;calculate(for:);;;Argument[0];weak-password-hash-input-SHA2",
+        ";SHA3;true;callAsFunction(_:);;;Argument[0];weak-password-hash-input-SHA2",
+        ";SHA3;true;process(block:currentHash:);;;Argument[0];weak-password-hash-input-SHA2",
+        ";SHA3;true;update(withBytes:isLast:);;;Argument[0];weak-password-hash-input-SHA2",
+        ";Digest;true;sha2(_:variant:);;;Argument[0];weak-password-hash-input-SHA2",
+        ";Digest;true;sha3(_:variant:);;;Argument[0];weak-password-hash-input-SHA3",
+        ";Digest;true;sha224(_:);;;Argument[0];weak-password-hash-input-SHA224",
+        ";Digest;true;sha256(_:);;;Argument[0];weak-password-hash-input-SHA256",
+        ";Digest;true;sha384(_:);;;Argument[0];weak-password-hash-input-SHA384",
+        ";Digest;true;sha512(_:);;;Argument[0];weak-password-hash-input-SHA512",
+        ";Array;true;sha2(_:);;;Argument[-1];weak-password-hash-input-SHA2",
+        ";Array;true;sha3(_:);;;Argument[-1];weak-password-hash-input-SHA3",
+        ";Array;true;sha224();;;Argument[-1];weak-password-hash-input-SHA224",
+        ";Array;true;sha256();;;Argument[-1];weak-password-hash-input-SHA256",
+        ";Array;true;sha384();;;Argument[-1];weak-password-hash-input-SHA384",
+        ";Array;true;sha512();;;Argument[-1];weak-password-hash-input-SHA512",
+        ";Data;true;sha2(_:);;;Argument[-1];weak-password-hash-input-SHA2",
+        ";Data;true;sha3(_:);;;Argument[-1];weak-password-hash-input-SHA3",
+        ";Data;true;sha224();;;Argument[-1];weak-password-hash-input-SHA224",
+        ";Data;true;sha256();;;Argument[-1];weak-password-hash-input-SHA256",
+        ";Data;true;sha384();;;Argument[-1];weak-password-hash-input-SHA384",
+        ";Data;true;sha512();;;Argument[-1];weak-password-hash-input-SHA512",
+        ";String;true;sha2(_:);;;Argument[-1];weak-password-hash-input-SHA2",
+        ";String;true;sha3(_:);;;Argument[-1];weak-password-hash-input-SHA3",
+        ";String;true;sha224();;;Argument[-1];weak-password-hash-input-SHA224",
+        ";String;true;sha256();;;Argument[-1];weak-password-hash-input-SHA256",
+        ";String;true;sha384();;;Argument[-1];weak-password-hash-input-SHA384",
+        ";String;true;sha512();;;Argument[-1];weak-password-hash-input-SHA512",
+      ]
+  }
+}
+
+/**
+ * A sink defined in a CSV model.
+ */
+private class DefaultWeakPasswordHashingSink extends WeakPasswordHashingSink {
+  string algorithm;
+
+  DefaultWeakPasswordHashingSink() { sinkNode(this, "weak-password-hash-input-" + algorithm) }
+
+  override string getAlgorithm() { result = algorithm }
+}
+
+/**
+ * A barrier for weak password hashing, when it occurs inside of
+ * certain cryptographic algorithms as part of their design.
+ */
+class WeakPasswordHashingImplementationBarrier extends WeakPasswordHashingBarrier {
+  WeakPasswordHashingImplementationBarrier() {
+    this.asParameter()
+        .getDeclaringFunction()
+        .(Function)
+        .getDeclaringDecl*()
+        .(NominalTypeDecl)
+        .getName() = ["HMAC", "PBKDF1", "PBKDF2"]
+  }
+}

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

@@ -0,0 +1,43 @@
+/**
+ * Provides a taint tracking configuration to find use of inappropriate
+ * cryptographic hashing algorithms on passwords.
+ */
+
+import swift
+import codeql.swift.security.SensitiveExprs
+import codeql.swift.dataflow.DataFlow
+import codeql.swift.dataflow.TaintTracking
+import codeql.swift.security.WeakPasswordHashingExtensions
+
+/**
+ * A taint tracking configuration from password expressions to inappropriate
+ * hashing sinks.
+ */
+module WeakPasswordHashingConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) {
+    exists(SensitiveExpr se |
+      node.asExpr() = se and
+      se.getSensitiveType() instanceof SensitivePassword
+    )
+  }
+
+  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)
+  }
+
+  predicate isBarrierOut(DataFlow::Node node) {
+    // 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 WeakPasswordHashingFlow = TaintTracking::Global<WeakPasswordHashingConfig>;

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

@@ -4,7 +4,6 @@
  */
 
 import swift
-import codeql.swift.security.SensitiveExprs
 import codeql.swift.dataflow.DataFlow
 import codeql.swift.dataflow.ExternalFlow
 
@@ -35,7 +34,7 @@ class WeakSensitiveDataHashingAdditionalFlowStep extends Unit {
   abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
 }
 
-private class WeakHashingSinks extends SinkModelCsv {
+private class WeakSensitiveDataHashingSinks extends SinkModelCsv {
   override predicate row(string row) {
     row =
       [
@@ -49,9 +48,11 @@ private class WeakHashingSinks extends SinkModelCsv {
         // CryptoSwift
         ";MD5;true;calculate(for:);;;Argument[0];weak-hash-input-MD5",
         ";MD5;true;callAsFunction(_:);;;Argument[0];weak-hash-input-MD5",
+        ";MD5;true;process(block:currentHash:);;;Argument[0];weak-hash-input-MD5",
         ";MD5;true;update(withBytes:isLast:);;;Argument[0];weak-hash-input-MD5",
         ";SHA1;true;calculate(for:);;;Argument[0];weak-hash-input-SHA1",
         ";SHA1;true;callAsFunction(_:);;;Argument[0];weak-hash-input-SHA1",
+        ";SHA1;true;process(block:currentHash:);;;Argument[0];weak-hash-input-SHA1",
         ";SHA1;true;update(withBytes:isLast:);;;Argument[0];weak-hash-input-SHA1",
         ";Digest;true;md5(_:);;;Argument[0];weak-hash-input-MD5",
         ";Digest;true;sha1(_:);;;Argument[0];weak-hash-input-SHA1",
@@ -68,10 +69,10 @@ private class WeakHashingSinks extends SinkModelCsv {
 /**
  * A sink defined in a CSV model.
  */
-private class DefaultWeakHashingSink extends WeakSensitiveDataHashingSink {
+private class DefaultWeakSenitiveDataHashingSink extends WeakSensitiveDataHashingSink {
   string algorithm;
 
-  DefaultWeakHashingSink() { sinkNode(this, "weak-hash-input-" + algorithm) }
+  DefaultWeakSenitiveDataHashingSink() { sinkNode(this, "weak-hash-input-" + algorithm) }
 
   override string getAlgorithm() { result = algorithm }
 }

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

@@ -13,8 +13,13 @@ import codeql.swift.security.WeakSensitiveDataHashingExtensions
  * A taint tracking configuration from sensitive expressions to broken or weak
  * hashing sinks.
  */
-module WeakHashingConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr }
+module WeakSensitiveDataHashingConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) {
+    exists(SensitiveExpr se |
+      node.asExpr() = se and
+      not se.getSensitiveType() instanceof SensitivePassword // responsibility of the weak password hashing query
+    )
+  }
 
   predicate isSink(DataFlow::Node node) { node instanceof WeakSensitiveDataHashingSink }
 
@@ -35,4 +40,8 @@ module WeakHashingConfig implements DataFlow::ConfigSig {
   }
 }
 
-module WeakHashingFlow = TaintTracking::Global<WeakHashingConfig>;
+deprecated module WeakHashingConfig = WeakSensitiveDataHashingConfig;
+
+module WeakSensitiveDataHashingFlow = TaintTracking::Global<WeakSensitiveDataHashingConfig>;
+
+deprecated module WeakHashingFlow = WeakSensitiveDataHashingFlow;

swift/ql/src/change-notes/2023-12-15-weak-password-hashing.md

@@ -0,0 +1,5 @@
+---
+category: newQuery
+---
+
+* Added new query "Use of an inappropriate cryptographic hashing algorithm on passwords" (`swift/weak-password-hashing`). This query detects use of inappropriate hashing algorithms for password hashing. Some of the results of this query are new, others would previously have been reported by the "Use of a broken or weak cryptographic hashing algorithm on sensitive data" (`swift/weak-sensitive-data-hashing`) query.