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,113 @@
+/**
+ * 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 }
+}

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/queries/Security/CWE-328/WeakPasswordHashing.qhelp

@@ -0,0 +1,93 @@
+<!DOCTYPE qhelp PUBLIC
+        "-//Semmle//qhelp//EN"
+        "qhelp.dtd">
+<qhelp>
+    <overview>
+        <p>
+            Using a insufficiently computationally hard hash function can leave data
+            vulnerable, and should not be used for password hashing.
+        </p>
+
+        <p>
+            A strong cryptographic hash function should be resistant to:
+        </p>
+        <ul>
+            <li>
+                <strong>Pre-image attacks</strong>. If you know a hash value <code>h(x)</code>,
+                you should not be able to easily find the input <code>x</code>.
+            </li>
+            <li>
+                <strong>Collision attacks</strong>. If you know a hash value <code>h(x)</code>,
+                you should not be able to easily find a different input
+                <code>y</code>
+                with the same hash value <code>h(x) = h(y)</code>.
+            </li>
+            <li>
+                <strong>Brute force</strong>. If you know a hash value <code>h(x)</code>,
+                you should not be able to find an input <code>y</code> that computes to that hash value
+                using brute force attacks without significant computational effort.
+            <li>
+        </ul>
+
+        <p>
+            All of MD5, SHA-1, SHA-2 and SHA-3 are weak against offline brute forcing, since they are not computationally hard. This includes SHA-224, SHA-256, SHA-384 and SHA-512, which are in the SHA-2 family.
+        </p>
+
+        <p>
+            Password hashing algorithms are designed to be slow and/or memory intenstive to compute, which makes brute force attacks more difficult.
+        </p>
+
+    </overview>
+    <recommendation>
+
+        <p>
+            Ensure that for password storage you should use a computationally hard cryptographic hash function, such as:
+        </p>
+
+        <ul>
+            <li>
+                Argon2
+            </li>
+            <li>
+                scrypt
+            </li>
+            <li>
+                bcrypt
+            </li>
+            <li>
+                PBKDF2
+            </li>
+        </ul>
+
+    </recommendation>
+    <example>
+
+        <p>
+            The following examples show a function that hashes a password using a cryptographic hashing algorithm.
+
+            In the first case the SHA-512 hashing algorithm is used. It is vulnerable to offline brute force attacks:
+        </p>
+        <sample src="WeakPasswordHashingBad.swift"/>
+        <p>
+
+            Here is the same function using Argon2, which is suitable for password hashing:
+        </p>
+        <sample src="WeakPasswordHashingGood.swift"/>
+
+    </example>
+    <references>
+        <li>
+            OWASP:
+            <a href="https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html">Password Storage
+                Cheat Sheet
+            </a>
+        </li>
+        <li>
+            libsodium: <a href="https://doc.libsodium.org/bindings_for_other_languages#bindings-programming-languages">libsodium bindings for other languages</a>
+        </li>
+        <li>
+            GitHub: <a href="https://github.com/tmthecoder/Argon2Swift">Argon2Swift</a>
+        </li>
+    </references>
+
+</qhelp>