Query to detect hash without salt

csharp/ql/src/experimental/Security Features/CWE-759/HashWithoutSalt.cs

@@ -0,0 +1,74 @@
+// semmle-extractor-options: /r:System.Security.Cryptography.Primitives.dll /r:System.Security.Cryptography.Csp.dll /r:System.Security.Cryptography.Algorithms.dll
+
+using System;
+using System.Security.Cryptography;
+
+using Windows.Security.Cryptography;
+using Windows.Security.Cryptography.Core;
+using Windows.Storage.Streams;
+
+public class Test
+{
+    private const int SaltSize = 32;
+
+    // BAD - Hash without a salt.
+    public static String HashPassword(string password, string strAlgName ="SHA256")
+    {
+        IBuffer passBuff = CryptographicBuffer.ConvertStringToBinary(password, BinaryStringEncoding.Utf8);
+        HashAlgorithmProvider algProvider = HashAlgorithmProvider.OpenAlgorithm(strAlgName);
+        IBuffer hashBuff = algProvider.HashData(passBuff);
+        return CryptographicBuffer.EncodeToBase64String(hashBuff);
+    }
+
+    // GOOD - Hash with a salt.
+    public static string HashPassword2(string password, string salt, string strAlgName ="SHA256")
+    {
+        // Concatenate the salt with the password.
+        IBuffer passBuff = CryptographicBuffer.ConvertStringToBinary(password+salt, BinaryStringEncoding.Utf8);
+        HashAlgorithmProvider algProvider = HashAlgorithmProvider.OpenAlgorithm(strAlgName);
+        IBuffer hashBuff = algProvider.HashData(passBuff);
+        return CryptographicBuffer.EncodeToBase64String(hashBuff);
+    }
+
+    // BAD - Hash without a salt.
+    public static string HashPassword(string password)
+    {
+        SHA256 sha256Hash = SHA256.Create();
+        byte[] passBytes = System.Text.Encoding.ASCII.GetBytes(password);
+        byte[] hashBytes = sha256Hash.ComputeHash(passBytes);
+        return Convert.ToBase64String(hashBytes);
+    }
+
+    // GOOD - Hash with a salt.
+    public static string HashPassword2(string password)
+    {
+        byte[] passBytes = System.Text.Encoding.ASCII.GetBytes(password);
+        byte[] saltBytes = GenerateSalt();
+
+        // Add the salt to the hash.
+        byte[] rawSalted  = new byte[passBytes.Length + saltBytes.Length]; 
+        passBytes.CopyTo(rawSalted, 0);
+        saltBytes.CopyTo(rawSalted, passBytes.Length);
+
+        //Create the salted hash.         
+        SHA256 sha256 = SHA256.Create();
+        byte[] saltedPassBytes = sha256.ComputeHash(rawSalted);
+
+        // Add the salt value to the salted hash.
+        byte[] dbPassword  = new byte[saltedPassBytes.Length + saltBytes.Length];
+        saltedPassBytes.CopyTo(dbPassword, 0);
+        saltBytes.CopyTo(dbPassword, saltedPassBytes.Length);
+
+        return Convert.ToBase64String(dbPassword);
+    }
+
+    public static byte[] GenerateSalt()
+    {
+        using (var rng = new RNGCryptoServiceProvider())
+        {
+            var randomNumber = new byte[SaltSize];
+            rng.GetBytes(randomNumber);
+            return randomNumber;
+        }
+    }
+}

csharp/ql/src/experimental/Security Features/CWE-759/HashWithoutSalt.expected

@@ -0,0 +1,13 @@
+edges
+| HashWithoutSalt.cs:17:70:17:77 | access to parameter password : String | HashWithoutSalt.cs:19:49:19:56 | access to local variable passBuff |
+| HashWithoutSalt.cs:37:28:37:72 | call to method GetBytes : Byte[] | HashWithoutSalt.cs:38:51:38:59 | access to local variable passBytes |
+| HashWithoutSalt.cs:37:64:37:71 | access to parameter password : String | HashWithoutSalt.cs:37:28:37:72 | call to method GetBytes : Byte[] |
+nodes
+| HashWithoutSalt.cs:17:70:17:77 | access to parameter password : String | semmle.label | access to parameter password : String |
+| HashWithoutSalt.cs:19:49:19:56 | access to local variable passBuff | semmle.label | access to local variable passBuff |
+| HashWithoutSalt.cs:37:28:37:72 | call to method GetBytes : Byte[] | semmle.label | call to method GetBytes : Byte[] |
+| HashWithoutSalt.cs:37:64:37:71 | access to parameter password : String | semmle.label | access to parameter password : String |
+| HashWithoutSalt.cs:38:51:38:59 | access to local variable passBytes | semmle.label | access to local variable passBytes |
+#select
+| HashWithoutSalt.cs:19:49:19:56 | access to local variable passBuff | HashWithoutSalt.cs:17:70:17:77 | access to parameter password : String | HashWithoutSalt.cs:19:49:19:56 | access to local variable passBuff | $@ is hashed without a salt. | HashWithoutSalt.cs:17:70:17:77 | access to parameter password | The password |
+| HashWithoutSalt.cs:38:51:38:59 | access to local variable passBytes | HashWithoutSalt.cs:37:64:37:71 | access to parameter password : String | HashWithoutSalt.cs:38:51:38:59 | access to local variable passBytes | $@ is hashed without a salt. | HashWithoutSalt.cs:37:64:37:71 | access to parameter password | The password |

csharp/ql/src/experimental/Security Features/CWE-759/HashWithoutSalt.qhelp

@@ -0,0 +1,29 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+  <overview>
+    <p>In cryptography, a salt is some random data used as an additional input to a one-way function that hashes a password or pass-phrase. It makes dictionary attacks more difficult.</p>
+
+    <p>Without a salt, it is much easier for attackers to pre-compute the hash value using dictionary attack techniques such as rainbow tables to crack passwords.</p>
+  </overview>
+
+  <recommendation>
+    <p>Use a long random salt of at least 32 bytes then use the combination of password and salt to hash a password or password phrase.</p>
+  </recommendation>
+
+  <example>
+    <p>The following example shows two ways of hashing. In the 'BAD' cases, no salt is provided. In the 'GOOD' cases, a salt is provided.</p>
+    <sample src="HashWithoutSalt.cs" />
+  </example>
+
+  <references>
+    <li>
+      DZone:
+      <a href="https://dzone.com/articles/a-look-at-java-cryptography">A Look at Java Cryptography</a>
+    </li>
+    <li>
+      CWE:
+      <a href="https://cwe.mitre.org/data/definitions/759.html">CWE-759: Use of a One-Way Hash without a Salt</a>
+    </li>
+  </references>
+</qhelp>

csharp/ql/src/experimental/Security Features/CWE-759/HashWithoutSalt.ql

@@ -0,0 +1,98 @@
+/**
+ * @name Use of a hash function without a salt
+ * @description Hashed passwords without a salt are vulnerable to dictionary attacks.
+ * @kind path-problem
+ * @id cs/hash-without-salt
+ * @tags security
+ *       external/cwe-759
+ */
+
+import csharp
+import semmle.code.csharp.dataflow.TaintTracking
+import DataFlow::PathGraph
+
+/** The C# class `System.Security.Cryptography.SHA...` other than the weak `SHA1`. */
+class SHA extends RefType {
+  SHA() { this.getQualifiedName().regexpMatch("System\\.Security\\.Cryptography\\.SHA\\d{2,3}") }
+}
+
+class HashAlgorithmProvider extends RefType {
+  HashAlgorithmProvider() {
+    this.hasQualifiedName("Windows.Security.Cryptography.Core", "HashAlgorithmProvider")
+  }
+}
+
+/** The method call `ComputeHash()` declared in `System.Security.Cryptography.SHA...`. */
+class ComputeHashMethodCall extends MethodCall {
+  ComputeHashMethodCall() {
+    this.getQualifier().getType() instanceof SHA and
+    this.getTarget().hasName("ComputeHash")
+  }
+}
+
+/** The method call `ComputeHash()` declared in `System.Security.Cryptography.SHA...`. */
+class HashDataMethodCall extends MethodCall {
+  HashDataMethodCall() {
+    this.getQualifier().getType() instanceof HashAlgorithmProvider and
+    this.getTarget().hasName("HashData")
+  }
+}
+
+/** Gets a regular expression for matching common names of variables that indicate the value being held is a password. */
+string getPasswordRegex() { result = "(?i).*pass(wd|word|code|phrase).*" }
+
+/** Finds variables that hold password information judging by their names. */
+class PasswordVarExpr extends Expr {
+  PasswordVarExpr() {
+    exists(Variable v | this = v.getAnAccess() | v.getName().regexpMatch(getPasswordRegex()))
+  }
+}
+
+/** Taint configuration tracking flow from an expression whose name suggests it holds password data to a method call that generates a hash without a salt. */
+class HashWithoutSaltConfiguration extends TaintTracking::Configuration {
+  HashWithoutSaltConfiguration() { this = "HashWithoutSaltConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof PasswordVarExpr }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(ComputeHashMethodCall mc |
+      sink.asExpr() = mc.getArgument(0) // sha256Hash.ComputeHash(rawDatabytes)
+    ) or
+    exists(HashDataMethodCall mc |
+      sink.asExpr() = mc.getArgument(0) // algProv.HashData(rawDatabytes)
+    )
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    exists(MethodCall mc |
+      mc.getTarget()
+          .hasQualifiedName("Windows.Security.Cryptography.CryptographicBuffer",
+            "ConvertStringToBinary") and
+      mc.getArgument(0) = node1.asExpr() and
+      mc = node2.asExpr()
+    )
+  }
+
+  /**
+   * Holds if a password is concatenated with a salt then hashed together through the call `System.Array.CopyTo()`, for example,
+   *  `byte[] rawSalted  = new byte[passBytes.Length + salt.Length];`
+   *  `passBytes.CopyTo(rawSalted, 0);`
+   *  `salt.CopyTo(rawSalted, passBytes.Length);`
+   *  `byte[] saltedPassword = sha256.ComputeHash(rawSalted);`
+   *  Or the password is concatenated with a salt as a string.
+   */
+  override predicate isSanitizer(DataFlow::Node node) {
+    exists(MethodCall mc |
+      mc.getTarget().fromLibrary() and
+      mc.getTarget().hasQualifiedName("System.Array", "CopyTo") and
+      mc.getArgument(0) = node.asExpr()
+    ) // passBytes.CopyTo(rawSalted, 0)
+    or
+    exists(AddExpr e | node.asExpr() = e.getAnOperand()) // password+salt
+  }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, HashWithoutSaltConfiguration c
+where c.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "$@ is hashed without a salt.", source.getNode(),
+  "The password"

csharp/ql/src/experimental/Security Features/CWE-759/Stubs.cs

@@ -0,0 +1,45 @@
+namespace Windows.Security.Cryptography
+{
+    public enum BinaryStringEncoding
+    {
+        Utf8,
+        Utf16LE,
+        Utf16BE
+    }
+
+    public static class CryptographicBuffer
+    {
+        public static Windows.Storage.Streams.IBuffer ConvertStringToBinary(string value, BinaryStringEncoding encoding) => throw null;
+
+        public static string EncodeToBase64String(Windows.Storage.Streams.IBuffer buffer) => throw null;
+    }
+}
+
+namespace Windows.Storage.Streams
+{
+    public interface IBuffer {
+        public uint Capacity { get; }
+
+        public uint Length { get; set; }
+     }
+}
+
+namespace Windows.Security.Cryptography.Core
+{
+    public sealed class CryptographicKey { }
+
+    public sealed class SymmetricKeyAlgorithmProvider
+    {
+        public CryptographicKey CreateSymmetricKey(Windows.Storage.Streams.IBuffer keyMaterial) => throw null;
+    }
+
+    public sealed class HashAlgorithmProvider {
+        public string AlgorithmName { get; }
+
+        public uint HashLength { get; }
+
+        public static HashAlgorithmProvider OpenAlgorithm(string algorithm) => throw null;
+
+        public Windows.Storage.Streams.IBuffer HashData(Windows.Storage.Streams.IBuffer data) => throw null;
+    }
+}