PS: Weak kdf query

powershell/ql/src/queries/security/cwe-327/WeakKDFConfiguration.qhelp

@@ -0,0 +1,40 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+When deriving cryptographic keys from passwords using <code>Rfc2898DeriveBytes</code> (PBKDF2),
+both the iteration count and hash algorithm must be configured securely.
+An insufficient iteration count or a weak hash algorithm makes the derived key
+vulnerable to brute-force attacks.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Always specify at least 100,000 iterations and use SHA-256 or a stronger hash algorithm
+(SHA-384, SHA-512) when creating an <code>Rfc2898DeriveBytes</code> instance or calling the
+static <code>Pbkdf2</code> method.
+</p>
+</recommendation>
+
+<example>
+<p>The following example shows insecure usage with default settings:</p>
+<sample src="examples/WeakKDFConfigurationBad.ps1" />
+<p>The following example shows secure usage with adequate iterations and a strong hash:</p>
+<sample src="examples/WeakKDFConfigurationGood.ps1" />
+</example>
+
+<references>
+<li>
+  OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html">Password Storage Cheat Sheet</a>
+</li>
+<li>
+  Microsoft: <a href="https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.rfc2898derivebytes">Rfc2898DeriveBytes Class</a>
+</li>
+<li>
+  CWE-327: <a href="https://cwe.mitre.org/data/definitions/327.html">Use of a Broken or Risky Cryptographic Algorithm</a>
+</li>
+</references>
+</qhelp>

powershell/ql/src/queries/security/cwe-327/WeakKDFConfiguration.ql

@@ -0,0 +1,213 @@
+/**
+ * @name Weak key derivation function configuration
+ * @description Rfc2898DeriveBytes (PBKDF2) should use at least 100,000 iterations
+ *              and a hash algorithm of SHA-256 or stronger to resist brute-force attacks.
+ * @kind problem
+ * @problem.severity error
+ * @security-severity 7.5
+ * @precision high
+ * @id powershell/weak-kdf-configuration
+ * @tags security
+ *       external/cwe/cwe-327
+ *       external/cwe/cwe-328
+ *       cryptography
+ */
+
+import powershell
+import semmle.code.powershell.ApiGraphs
+import semmle.code.powershell.dataflow.DataFlow
+
+private int minIterationCount() { result = 100000 }
+
+/**
+ * An instantiation of Rfc2898DeriveBytes via New-Object or [Type]::new().
+ */
+class Rfc2898DeriveBytesCreation extends DataFlow::CallNode {
+  Rfc2898DeriveBytesCreation() {
+    this =
+      API::getTopLevelMember("system")
+          .getMember("security")
+          .getMember("cryptography")
+          .getMember("rfc2898derivebytes")
+          .getMember("new")
+          .asCall()
+    or
+    // New-Object pattern
+    exists(DataFlow::ObjectCreationNode oc |
+      oc = this and
+      oc.getLowerCaseConstructedTypeName() =
+        [
+          "system.security.cryptography.rfc2898derivebytes",
+          "rfc2898derivebytes"
+        ]
+    )
+  }
+
+  private DataFlow::Node getNewObjectArgumentList() {
+    this.getExprNode().getExpr() instanceof DotNetObjectCreation and
+    (
+      result = this.getNamedArgument("argumentlist")
+      or
+      not this.hasNamedArgument("argumentlist") and result = this.getPositionalArgument(1)
+    )
+  }
+
+  private DataFlow::Node getNewObjectArgument(int index) {
+    exists(ArrayLiteral args |
+      args = this.getNewObjectArgumentList().asExpr().getExpr() and
+      result.asExpr().getExpr() = args.getExpr(index)
+    )
+    or
+    exists(ParenExpr paren, ArrayLiteral args |
+      paren = this.getNewObjectArgumentList().asExpr().getExpr() and
+      args = paren.getExpr() and
+      result.asExpr().getExpr() = args.getExpr(index)
+    )
+  }
+
+  private predicate hasKnownNewObjectArgumentList() {
+    this.getNewObjectArgumentList().asExpr().getExpr() instanceof ArrayLiteral
+    or
+    this.getNewObjectArgumentList().asExpr().getExpr().(ParenExpr).getExpr() instanceof ArrayLiteral
+  }
+
+  private DataFlow::Node getConstructorArgument(int index) {
+    this.getExprNode().getExpr() instanceof NewObjectCreation and
+    result = this.getPositionalArgument(index)
+    or
+    result = this.getNewObjectArgument(index)
+  }
+
+  /** Gets the iteration count argument (position 2, 0-indexed), if any. */
+  DataFlow::Node getIterationCountArg() { result = this.getConstructorArgument(2) }
+
+  /** Gets the hash algorithm argument (position 3, 0-indexed), if any. */
+  DataFlow::Node getHashAlgorithmArg() { result = this.getConstructorArgument(3) }
+
+  /** Holds if the constructor is known to omit the iteration count argument. */
+  predicate hasDefaultIterationCount() {
+    not this.getExprNode().getExpr() instanceof DotNetObjectCreation and
+    not exists(this.getIterationCountArg())
+    or
+    this.hasKnownNewObjectArgumentList() and not exists(this.getNewObjectArgument(2))
+  }
+
+  /** Holds if the constructor is known to omit the hash algorithm argument. */
+  predicate hasDefaultHashAlgorithm() {
+    not this.getExprNode().getExpr() instanceof DotNetObjectCreation and
+    not exists(this.getHashAlgorithmArg())
+    or
+    this.hasKnownNewObjectArgumentList() and not exists(this.getNewObjectArgument(3))
+  }
+}
+
+/**
+ * A call to the static Rfc2898DeriveBytes.Pbkdf2 method (.NET 6+).
+ */
+class Pbkdf2StaticCall extends DataFlow::CallNode {
+  Pbkdf2StaticCall() {
+    this =
+      API::getTopLevelMember("system")
+          .getMember("security")
+          .getMember("cryptography")
+          .getMember("rfc2898derivebytes")
+          .getMember("pbkdf2")
+          .asCall()
+  }
+
+  /** Gets the iteration count argument (position 2, 0-indexed). */
+  DataFlow::Node getIterationCountArg() { result = this.getPositionalArgument(2) }
+
+  /** Gets the hash algorithm argument (position 3, 0-indexed). */
+  DataFlow::Node getHashAlgorithmArg() { result = this.getPositionalArgument(3) }
+}
+
+/**
+ * Holds if `node` is an integer literal less than the minimum iteration count.
+ */
+private predicate isLowIterationValue(DataFlow::Node node, int value) {
+  value = node.asExpr().getExpr().getValue().asInt() and
+  value < minIterationCount()
+}
+
+/**
+ * Holds if `node` references a weak hash algorithm (MD5 or SHA1).
+ */
+private predicate isWeakHashAlgorithm(DataFlow::Node node, string name) {
+  // [HashAlgorithmName]::MD5 or [HashAlgorithmName]::SHA1
+  node =
+    API::getTopLevelMember("system")
+        .getMember("security")
+        .getMember("cryptography")
+        .getMember("hashalgorithmname")
+        .getMember(name)
+        .asSource() and
+  name = ["md5", "sha1"]
+  or
+  // String literal "MD5" or "SHA1"
+  exists(string s |
+    s = node.asExpr().getExpr().getValue().asString().toLowerCase() and
+    s = ["md5", "sha1"] and
+    name = s
+  )
+}
+
+/**
+ * Gets a human-readable name for the weak hash algorithm.
+ */
+private string prettyHashName(string name) {
+  name = "md5" and result = "MD5"
+  or
+  name = "sha1" and result = "SHA1"
+}
+
+from DataFlow::CallNode call, string message
+where
+  // Case 1: Rfc2898DeriveBytes created without specifying iteration count
+  call instanceof Rfc2898DeriveBytesCreation and
+  call.(Rfc2898DeriveBytesCreation).hasDefaultIterationCount() and
+  message =
+    "Rfc2898DeriveBytes uses default iteration count of 1000. Specify at least " +
+      minIterationCount().toString() + " iterations."
+  or
+  // Case 2: Rfc2898DeriveBytes created with low iteration count
+  exists(int value |
+    call instanceof Rfc2898DeriveBytesCreation and
+    isLowIterationValue(call.(Rfc2898DeriveBytesCreation).getIterationCountArg(), value) and
+    message =
+      "Rfc2898DeriveBytes uses iteration count of " + value.toString() +
+        ", which is below the minimum of " + minIterationCount().toString() + "."
+  )
+  or
+  // Case 3: Rfc2898DeriveBytes created without specifying hash algorithm (defaults to SHA1)
+  call instanceof Rfc2898DeriveBytesCreation and
+  call.(Rfc2898DeriveBytesCreation).hasDefaultHashAlgorithm() and
+  message = "Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger."
+  or
+  // Case 4: Rfc2898DeriveBytes created with weak hash algorithm
+  exists(string name |
+    call instanceof Rfc2898DeriveBytesCreation and
+    isWeakHashAlgorithm(call.(Rfc2898DeriveBytesCreation).getHashAlgorithmArg(), name) and
+    message =
+      "Rfc2898DeriveBytes uses weak hash algorithm " + prettyHashName(name) +
+        ". Use SHA-256 or stronger."
+  )
+  or
+  // Case 5: Pbkdf2 static method with low iteration count
+  exists(int value |
+    call instanceof Pbkdf2StaticCall and
+    isLowIterationValue(call.(Pbkdf2StaticCall).getIterationCountArg(), value) and
+    message =
+      "Rfc2898DeriveBytes.Pbkdf2 uses iteration count of " + value.toString() +
+        ", which is below the minimum of " + minIterationCount().toString() + "."
+  )
+  or
+  // Case 6: Pbkdf2 static method with weak hash algorithm
+  exists(string name |
+    call instanceof Pbkdf2StaticCall and
+    isWeakHashAlgorithm(call.(Pbkdf2StaticCall).getHashAlgorithmArg(), name) and
+    message =
+      "Rfc2898DeriveBytes.Pbkdf2 uses weak hash algorithm " + prettyHashName(name) +
+        ". Use SHA-256 or stronger."
+  )
+select call, message

powershell/ql/src/queries/security/cwe-327/examples/WeakKDFConfigurationBad.ps1

@@ -0,0 +1,8 @@
+# BAD: Default iteration count (1000) and default hash algorithm (SHA1)
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt)
+
+# BAD: Low iteration count
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 1000)
+
+# BAD: Weak hash algorithm
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 100000, [System.Security.Cryptography.HashAlgorithmName]::SHA1)

powershell/ql/src/queries/security/cwe-327/examples/WeakKDFConfigurationGood.ps1

@@ -0,0 +1,5 @@
+# GOOD: 100,000+ iterations with SHA-256
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 600000, [System.Security.Cryptography.HashAlgorithmName]::SHA256)
+
+# GOOD: Static Pbkdf2 with strong configuration
+$key = [System.Security.Cryptography.Rfc2898DeriveBytes]::Pbkdf2($password, $salt, 600000, "SHA256", 32)

powershell/ql/test/query-tests/security/cwe-327/WeakKDFConfiguration/WeakKDFConfiguration.expected

@@ -0,0 +1,25 @@
+| WeakKDFConfiguration.ps1:7:8:7:79 | Call to new | Rfc2898DeriveBytes uses default iteration count of 1000. Specify at least 100000 iterations. |
+| WeakKDFConfiguration.ps1:7:8:7:79 | Call to new | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
+| WeakKDFConfiguration.ps1:11:8:11:85 | Call to new | Rfc2898DeriveBytes uses iteration count of 1000, which is below the minimum of 100000. |
+| WeakKDFConfiguration.ps1:11:8:11:85 | Call to new | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
+| WeakKDFConfiguration.ps1:14:8:14:86 | Call to new | Rfc2898DeriveBytes uses iteration count of 10000, which is below the minimum of 100000. |
+| WeakKDFConfiguration.ps1:14:8:14:86 | Call to new | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
+| WeakKDFConfiguration.ps1:17:8:17:86 | Call to new | Rfc2898DeriveBytes uses iteration count of 50000, which is below the minimum of 100000. |
+| WeakKDFConfiguration.ps1:17:8:17:86 | Call to new | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
+| WeakKDFConfiguration.ps1:21:8:21:87 | Call to new | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
+| WeakKDFConfiguration.ps1:25:8:25:143 | Call to new | Rfc2898DeriveBytes uses weak hash algorithm SHA1. Use SHA-256 or stronger. |
+| WeakKDFConfiguration.ps1:28:8:28:142 | Call to new | Rfc2898DeriveBytes uses weak hash algorithm MD5. Use SHA-256 or stronger. |
+| WeakKDFConfiguration.ps1:32:8:32:89 | Call to new-object | Rfc2898DeriveBytes uses iteration count of 5000, which is below the minimum of 100000. |
+| WeakKDFConfiguration.ps1:32:8:32:89 | Call to new-object | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
+| WeakKDFConfiguration.ps1:36:8:36:83 | Call to new-object | Rfc2898DeriveBytes uses default iteration count of 1000. Specify at least 100000 iterations. |
+| WeakKDFConfiguration.ps1:36:8:36:83 | Call to new-object | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
+| WeakKDFConfiguration.ps1:39:8:41:1 | Call to new-object | Rfc2898DeriveBytes uses iteration count of 5000, which is below the minimum of 100000. |
+| WeakKDFConfiguration.ps1:39:8:41:1 | Call to new-object | Rfc2898DeriveBytes uses weak hash algorithm SHA1. Use SHA-256 or stronger. |
+| WeakKDFConfiguration.ps1:42:8:44:1 | Call to new-object | Rfc2898DeriveBytes uses iteration count of 5000, which is below the minimum of 100000. |
+| WeakKDFConfiguration.ps1:42:8:44:1 | Call to new-object | Rfc2898DeriveBytes uses weak hash algorithm SHA1. Use SHA-256 or stronger. |
+| WeakKDFConfiguration.ps1:48:8:48:102 | Call to pbkdf2 | Rfc2898DeriveBytes.Pbkdf2 uses iteration count of 1000, which is below the minimum of 100000. |
+| WeakKDFConfiguration.ps1:52:8:52:102 | Call to pbkdf2 | Rfc2898DeriveBytes.Pbkdf2 uses weak hash algorithm SHA1. Use SHA-256 or stronger. |
+| WeakKDFConfiguration.ps1:55:8:55:84 | Call to new | Rfc2898DeriveBytes uses iteration count of 500, which is below the minimum of 100000. |
+| WeakKDFConfiguration.ps1:55:8:55:84 | Call to new | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
+| WeakKDFConfiguration.ps1:58:8:58:140 | Call to new | Rfc2898DeriveBytes uses iteration count of 500, which is below the minimum of 100000. |
+| WeakKDFConfiguration.ps1:58:8:58:140 | Call to new | Rfc2898DeriveBytes uses weak hash algorithm SHA1. Use SHA-256 or stronger. |

powershell/ql/test/query-tests/security/cwe-327/WeakKDFConfiguration/WeakKDFConfiguration.ps1

@@ -0,0 +1,99 @@
+# ===================================================================
+# ========== TRUE POSITIVES (should trigger alert) ==================
+# ===================================================================
+
+# --- Case 1: Default iteration count AND default algorithm ---
+# BAD: Only password and salt, defaults to 1000 iterations and SHA1
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt) # $ Alert $ Alert
+
+# --- Case 2: Low iteration count AND default algorithm ---
+# BAD: 1000 iterations is too low, and no algorithm specified
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 1000) # $ Alert $ Alert
+
+# BAD: 10000 iterations is still below 100k, and no algorithm specified
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 10000) # $ Alert $ Alert
+
+# BAD: 50000 iterations is still below 100k, and no algorithm specified
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 50000) # $ Alert $ Alert
+
+# --- Case 3: Default hash algorithm only (iterations are adequate) ---
+# BAD: Has iterations but no algorithm, defaults to SHA1
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 100000) # $ Alert
+
+# --- Case 4: Weak hash algorithm explicitly specified ---
+# BAD: SHA1 explicitly used
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 100000, [System.Security.Cryptography.HashAlgorithmName]::SHA1) # $ Alert
+
+# BAD: MD5 explicitly used
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 100000, [System.Security.Cryptography.HashAlgorithmName]::MD5) # $ Alert
+
+# --- Case 5: New-Object pattern (defaults to both issues) ---
+# BAD: New-Object with low iteration count and default algorithm
+$kdf = New-Object System.Security.Cryptography.Rfc2898DeriveBytes($password, $salt, 5000) # $ Alert $ Alert
+
+# --- Case 6: New-Object pattern with no extra args ---
+# BAD: New-Object with default iterations and algorithm
+$kdf = New-Object System.Security.Cryptography.Rfc2898DeriveBytes($password, $salt) # $ Alert $ Alert
+
+# BAD: Canonical New-Object forms with low iterations and weak algorithms
+$kdf = New-Object System.Security.Cryptography.Rfc2898DeriveBytes -ArgumentList (
+    $password, $salt, 5000, [System.Security.Cryptography.HashAlgorithmName]::SHA1
+) # $ Alert $ Alert
+$kdf = New-Object -TypeName System.Security.Cryptography.Rfc2898DeriveBytes -ArgumentList (
+    $password, $salt, 5000, [System.Security.Cryptography.HashAlgorithmName]::SHA1
+) # $ Alert $ Alert
+
+# --- Case 7: Pbkdf2 static method with low iterations ---
+# BAD: Static Pbkdf2 method with 1000 iterations
+$key = [System.Security.Cryptography.Rfc2898DeriveBytes]::Pbkdf2($password, $salt, 1000, "SHA256", 32) # $ Alert
+
+# --- Case 8: Pbkdf2 static method with weak hash ---
+# BAD: Static Pbkdf2 method with SHA1
+$key = [System.Security.Cryptography.Rfc2898DeriveBytes]::Pbkdf2($password, $salt, 100000, "SHA1", 32) # $ Alert
+
+# --- Case 9: Combined issues - low iterations AND no algorithm ---
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 500) # $ Alert $ Alert
+
+# --- Case 10: Combined issues - low iterations AND weak algorithm ---
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 500, [System.Security.Cryptography.HashAlgorithmName]::SHA1) # $ Alert $ Alert
+
+# ===================================================================
+# ========== TRUE NEGATIVES (should NOT trigger alert) ==============
+# ===================================================================
+
+# --- Safe: Proper iterations and strong hash ---
+# GOOD: 100000 iterations with SHA256
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 100000, [System.Security.Cryptography.HashAlgorithmName]::SHA256)
+
+# GOOD: 200000 iterations with SHA512
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 200000, [System.Security.Cryptography.HashAlgorithmName]::SHA512)
+
+# GOOD: 600000 iterations with SHA256
+$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 600000, [System.Security.Cryptography.HashAlgorithmName]::SHA256)
+
+# GOOD: Multiline New-Object calls from Encrypt-SycamoreBundle.ps1 and Decrypt-SycamoreBundle.ps1
+$encryptPbkdf2 = New-Object System.Security.Cryptography.Rfc2898DeriveBytes(
+    $passwordBytes, $salt, 100000, [System.Security.Cryptography.HashAlgorithmName]::SHA256
+)
+$decryptPbkdf2 = New-Object System.Security.Cryptography.Rfc2898DeriveBytes(
+    $passwordBytes, $salt, 100000, [System.Security.Cryptography.HashAlgorithmName]::SHA256
+)
+
+# GOOD: Canonical New-Object argument list forms
+$kdf = New-Object System.Security.Cryptography.Rfc2898DeriveBytes -ArgumentList (
+    $passwordBytes, $salt, 100000, [System.Security.Cryptography.HashAlgorithmName]::SHA256
+)
+$kdf = New-Object -TypeName System.Security.Cryptography.Rfc2898DeriveBytes -ArgumentList (
+    $passwordBytes, $salt, 100000, [System.Security.Cryptography.HashAlgorithmName]::SHA256
+)
+
+# GOOD: An opaque argument list is not assumed to use default settings
+$constructorArguments = @($passwordBytes, $salt, 100000, [System.Security.Cryptography.HashAlgorithmName]::SHA256)
+$kdf = New-Object -TypeName System.Security.Cryptography.Rfc2898DeriveBytes -ArgumentList $constructorArguments
+
+# --- Safe: Pbkdf2 with proper config ---
+# GOOD: Static Pbkdf2 with adequate iterations and strong hash
+$key = [System.Security.Cryptography.Rfc2898DeriveBytes]::Pbkdf2($password, $salt, 100000, "SHA256", 32)
+
+# GOOD: Static Pbkdf2 with high iterations
+$key = [System.Security.Cryptography.Rfc2898DeriveBytes]::Pbkdf2($password, $salt, 600000, "SHA512", 32)

powershell/ql/test/query-tests/security/cwe-327/WeakKDFConfiguration/WeakKDFConfiguration.qlref

@@ -0,0 +1,2 @@
+query: queries/security/cwe-327/WeakKDFConfiguration.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql