Flow from insecure random number to sensitive sink

powershell/ql/lib/semmle/code/powershell/frameworks/System.model.yml

@@ -1,4 +1,9 @@
 extensions:
+  - addsTo:
+      pack: microsoft/powershell-all
+      extensible: summaryModel
+    data:
+      - ["system.random", "Method[nextbytes]", "Argument[this]", "Argument[0]", "taint"]
   - addsTo:
       pack: microsoft/powershell-all
       extensible: sourceModel

powershell/ql/lib/semmle/code/powershell/frameworks/data/internal/ApiGraphModelsSpecific.qll

@@ -126,9 +126,20 @@ API::Node getExtraNodeFromType(string rawType) {
   or
   exists(string name, API::TypeNameNode typeName |
     parseRelevantType(rawType, name, _) and
-    typeName = API::getTopLevelMember(name) and
-    typeName.isImplicit() and
-    result = typeName
+    (
+      (
+        typeName.getTypeName() = name and
+        not typeName.isImplicit() and
+        result = typeName.getInstance()
+      )
+      or 
+      (
+        typeName = API::getTopLevelMember(name) and
+        typeName.isImplicit() and
+        result = typeName
+      )
+    )
+
   )
 }
 

powershell/ql/lib/semmle/code/powershell/security/InsecureRandomnessCustomizations.qll

@@ -0,0 +1,107 @@
+/**
+ * Provides default sources, sinks and sanitizers for reasoning about
+ * insecure-randomness vulnerabilities, as well as extension points for
+ * adding your own.
+ */
+
+private import semmle.code.powershell.dataflow.DataFlow
+import semmle.code.powershell.ApiGraphs
+
+module InsecureRandomness {
+  /**
+   * A data flow source for insecure-randomness vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for insecure-randomness vulnerabilities.
+   */
+  abstract class Sink extends DataFlow::Node {
+    /** Gets a human-readable description of this sink. */
+    abstract string getSinkDescription();
+  }
+
+  /**
+   * A sanitizer for insecure-randomness vulnerabilities.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /** A call to the `Get-Random` cmdlet. */
+  class GetRandomSource extends Source instanceof DataFlow::CallNode {
+    GetRandomSource() { this.matchesName("Get-Random") }
+  }
+
+  /** An instantiation of `System.Random` via `New-Object`. */
+  class SystemRandomObjectCreation extends Source instanceof DataFlow::ObjectCreationNode {
+    SystemRandomObjectCreation() {
+      this.asExpr()
+          .getExpr()
+          .(ObjectCreation)
+          .getAnArgument()
+          .getValue()
+          .stringMatches("System.Random")
+    }
+  }
+
+  /** A call to `[System.Random]::new()` via the API graph. */
+  class SystemRandomNewCall extends Source instanceof DataFlow::CallNode {
+    SystemRandomNewCall() {
+      this = API::getTopLevelMember("system").getMember("random").getMember("new").asCall()
+    }
+  }
+
+  /** Assignment to .Key or .IV on an object. */
+  class CryptoKeyIvAssignmentSink extends Sink {
+    CryptoKeyIvAssignmentSink() {
+      exists(AssignStmt a, MemberExprWriteAccess m |
+        m = a.getLeftHandSide() and
+        m.getLowerCaseMemberName() in ["key", "iv"] and
+        this.asExpr().getExpr() = a.getRightHandSide()
+      )
+    }
+
+    override string getSinkDescription() { result = "a cryptographic key or IV" }
+  }
+
+  /** Argument to a cryptographic constructor (HMAC key or KDF salt). */
+  class CryptoConstructorSink extends Sink {
+    CryptoConstructorSink() {
+      exists(DataFlow::ObjectCreationNode oc |
+        (
+          oc.getLowerCaseConstructedTypeName().matches("%hmac%")
+          or
+          oc.getLowerCaseConstructedTypeName().matches("%rfc2898derivebytes%")
+        ) and
+        this = oc.getAnArgument() and
+        not this = oc.getConstructedTypeNode()
+      )
+    }
+
+    override string getSinkDescription() { result = "a cryptographic operation" }
+  }
+
+  /** First positional argument to ConvertTo-SecureString. */
+  class SecureStringSink extends Sink {
+    SecureStringSink() {
+      exists(DataFlow::CallNode call |
+        call.matchesName("ConvertTo-SecureString") and
+        this = call.getPositionalArgument(0)
+      )
+    }
+
+    override string getSinkDescription() { result = "a secure string conversion" }
+  }
+
+  /** Value used in HTTP headers passed to Invoke-RestMethod or Invoke-WebRequest. */
+  class HttpHeaderValueSink extends Sink {
+    HttpHeaderValueSink() {
+      exists(DataFlow::CallNode call, HashTableExpr ht |
+        (call.matchesName("Invoke-RestMethod") or call.matchesName("Invoke-WebRequest")) and
+        call.getNamedArgument("headers").asExpr().getExpr() = ht and
+        this.asExpr().getExpr() = ht.getAValue()
+      )
+    }
+
+    override string getSinkDescription() { result = "an HTTP header" }
+  }
+}

powershell/ql/lib/semmle/code/powershell/security/InsecureRandomnessQuery.qll

@@ -0,0 +1,26 @@
+/**
+ * Provides a taint tracking configuration for reasoning about
+ * insecure-randomness vulnerabilities (CWE-338).
+ *
+ * Note, for performance reasons: only import this file if
+ * `InsecureRandomnessFlow` is needed, otherwise
+ * `InsecureRandomnessCustomizations` should be imported instead.
+ */
+
+import powershell
+import semmle.code.powershell.dataflow.TaintTracking
+import InsecureRandomnessCustomizations::InsecureRandomness
+import semmle.code.powershell.dataflow.DataFlow
+
+private module Config implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+}
+
+/**
+ * Taint-tracking for reasoning about insecure-randomness vulnerabilities.
+ */
+module InsecureRandomnessFlow = TaintTracking::Global<Config>;

powershell/ql/src/queries/security/cwe-338/InsecureRandomness.qhelp

@@ -0,0 +1,25 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+<overview>
+    <p>Using non-cryptographic random number generators for security-sensitive operations can compromise security. `Get-Random` and `System.Random` are not suitable for generating cryptographic keys, security tokens, or other security-critical random values.</p>
+</overview>
+
+<recommendation>
+    <p>Use a cryptographically secure random number generator such as `[System.Security.Cryptography.RandomNumberGenerator]` instead.</p>
+</recommendation>
+
+<example>
+    <p>Insecure random:</p>
+    <sample src="examples/InsecureRandomness/InsecureRandomnessBad.ps1" />
+
+    <p>Secure alternative:</p>
+    <sample src="examples/InsecureRandomness/InsecureRandomnessGood.ps1" />
+</example>
+
+<references>
+    <li><a href="https://cwe.mitre.org/data/definitions/338.html">CWE-338: Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG)</a>.</li>
+    <li><a href="https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.randomnumbergenerator">System.Security.Cryptography.RandomNumberGenerator</a>.</li>
+</references>
+
+</qhelp>

powershell/ql/src/queries/security/cwe-338/InsecureRandomness.ql

@@ -0,0 +1,23 @@
+/**
+ * @name Use of insecure random number generator in security-sensitive context
+ * @description Using non-cryptographic random number generators such as Get-Random or System.Random
+ *              for security-sensitive operations can compromise security.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.5
+ * @precision high
+ * @id powershell/insecure-randomness
+ * @tags security
+ *       external/cwe/cwe-330
+ *       external/cwe/cwe-338
+ */
+
+import powershell
+import semmle.code.powershell.security.InsecureRandomnessQuery
+import InsecureRandomnessFlow::PathGraph
+
+from InsecureRandomnessFlow::PathNode source, InsecureRandomnessFlow::PathNode sink
+where InsecureRandomnessFlow::flowPath(source, sink)
+select sink.getNode(), source, sink,
+  "Insecure random value flows to " + sink.getNode().(Sink).getSinkDescription() + " from $@.",
+  source.getNode(), "this insecure random source"

powershell/ql/src/queries/security/cwe-338/examples/InsecureRandomness/InsecureRandomnessBad.ps1

@@ -0,0 +1,42 @@
+# BAD: weak RNG output flows into security-sensitive .NET crypto sinks.
+
+# 1) Weak bytes flow into AES .Key and .IV properties
+$weak = New-Object System.Random
+$keyBytes = New-Object byte[] 32
+$ivBytes  = New-Object byte[] 16
+$weak.NextBytes($keyBytes)
+$weak.NextBytes($ivBytes)
+$aes = [System.Security.Cryptography.Aes]::Create()
+$aes.Key = $keyBytes   # BAD: predictable key
+$aes.IV  = $ivBytes    # BAD: predictable IV
+
+# 2) Weak bytes flow into HMAC constructor (signing key)
+$hmacKeyBytes = New-Object byte[] 64
+$weak.NextBytes($hmacKeyBytes)
+$hmac = New-Object System.Security.Cryptography.HMACSHA256(,$hmacKeyBytes) # BAD: predictable HMAC key
+
+# 3) Weak bytes used as salt for password-based key derivation
+$saltBytes = New-Object byte[] 8
+$weak.NextBytes($saltBytes)
+$kdf = New-Object System.Security.Cryptography.Rfc2898DeriveBytes("password", $saltBytes) # BAD: predictable salt
+
+# 4) Get-Random result flows into ConvertTo-SecureString → PSCredential
+$tempPwd = (Get-Random -Maximum 999999999).ToString()
+$securePwd = ConvertTo-SecureString $tempPwd -AsPlainText -Force
+$cred = New-Object System.Management.Automation.PSCredential("admin", $securePwd) # BAD: predictable credential
+
+# 5) Weak random flows into HTTP Authorization header
+$tokenValue = Get-Random -Maximum 999999999
+Invoke-RestMethod -Uri "https://api.example.com/data" `
+    -Headers @{ Authorization = "Bearer $tokenValue" } # BAD: predictable bearer token
+
+# 6) Weak random used as RNGCryptoServiceProvider substitute for key bytes
+$rng2 = [System.Random]::new()
+$aesKey2 = New-Object byte[] 32
+$rng2.NextBytes($aesKey2)
+[Security.Cryptography.RNGCryptoServiceProvider]::Create().GetBytes($aesKey2) | Out-Null
+# The above line re-fills the buffer, but the initial weak fill might persist
+# if code logic is wrong.  True BAD pattern: relying solely on System.Random:
+$aes2 = [System.Security.Cryptography.Aes]::Create()
+$rng2.NextBytes($aesKey2)     # BAD: final value comes from weak RNG
+$aes2.Key = $aesKey2          # BAD: predictable key assigned to cipher

powershell/ql/src/queries/security/cwe-338/examples/InsecureRandomness/InsecureRandomnessGood.ps1

@@ -0,0 +1,41 @@
+# GOOD: cryptographic RNG output flows into security-sensitive .NET crypto sinks.
+
+# 1) CSPRNG bytes flow into AES .Key and .IV properties
+$keyBytes = New-Object byte[] 32
+$ivBytes  = New-Object byte[] 16
+[System.Security.Cryptography.RandomNumberGenerator]::Fill($keyBytes)
+[System.Security.Cryptography.RandomNumberGenerator]::Fill($ivBytes)
+$aes = [System.Security.Cryptography.Aes]::Create()
+$aes.Key = $keyBytes   # GOOD: cryptographically random key
+$aes.IV  = $ivBytes    # GOOD: cryptographically random IV
+
+# 2) CSPRNG bytes flow into HMAC constructor (signing key)
+$hmacKeyBytes = New-Object byte[] 64
+[System.Security.Cryptography.RandomNumberGenerator]::Fill($hmacKeyBytes)
+$hmac = New-Object System.Security.Cryptography.HMACSHA256(,$hmacKeyBytes) # GOOD
+
+# 3) CSPRNG bytes used as salt for password-based key derivation
+$saltBytes = New-Object byte[] 16
+[System.Security.Cryptography.RandomNumberGenerator]::Fill($saltBytes)
+$kdf = New-Object System.Security.Cryptography.Rfc2898DeriveBytes("password", $saltBytes) # GOOD
+
+# 4) CSPRNG bytes flow into ConvertTo-SecureString → PSCredential
+$pwdBytes = New-Object byte[] 32
+[System.Security.Cryptography.RandomNumberGenerator]::Fill($pwdBytes)
+$tempPwd = [Convert]::ToBase64String($pwdBytes)
+$securePwd = ConvertTo-SecureString $tempPwd -AsPlainText -Force
+$cred = New-Object System.Management.Automation.PSCredential("admin", $securePwd) # GOOD
+
+# 5) CSPRNG bytes flow into HTTP Authorization header
+$tokenBytes = New-Object byte[] 32
+[System.Security.Cryptography.RandomNumberGenerator]::Fill($tokenBytes)
+$bearerToken = [Convert]::ToBase64String($tokenBytes)
+Invoke-RestMethod -Uri "https://api.example.com/data" `
+    -Headers @{ Authorization = "Bearer $bearerToken" } # GOOD
+
+# 6) CSPRNG via RNGCryptoServiceProvider fills key bytes for cipher
+$rng = [Security.Cryptography.RNGCryptoServiceProvider]::Create()
+$aesKey2 = New-Object byte[] 32
+$rng.GetBytes($aesKey2)       # GOOD: filled by CSPRNG
+$aes2 = [System.Security.Cryptography.Aes]::Create()
+$aes2.Key = $aesKey2          # GOOD: unpredictable key