Update ApprovedCipherMode query and tests

powershell/ql/lib/semmle/code/powershell/security/cryptography/CryptographyModule.qll

@@ -235,6 +235,32 @@ class CipherBlockModeEnum extends BlockMode {
   override string getName() { result = modeName }
 }
 
+private predicate cipherModeIntValue(int val, string name) {
+  val = 1 and name = "cbc"
+  or
+  val = 2 and name = "ecb"
+  or
+  val = 3 and name = "ofb"
+  or
+  val = 4 and name = "cfb"
+  or
+  val = 5 and name = "cts"
+}
+
+class CipherBlockModeIntConst extends BlockMode {
+  string modeName;
+
+  CipherBlockModeIntConst() {
+    exists(ConstExpr c, int val |
+      c = this.asExpr().getExpr() and
+      val = c.getValueString().toInt() and
+      cipherModeIntValue(val, modeName)
+    )
+  }
+
+  override string getName() { result = modeName }
+}
+
 class RsaCreateKeyCreation extends AsymmetricKeyCreation, DataFlow::CallNode {
   int keySize;
 

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

@@ -18,27 +18,59 @@ import semmle.code.powershell.ApiGraphs
 import semmle.code.powershell.security.cryptography.Concepts
 import WeakEncryptionFlow::PathGraph
 
-class AesModeProperty extends MemberExpr {
-  AesModeProperty() {
-    exists(DataFlow::ObjectCreationNode aesObjectCreation, DataFlow::Node aesObjectAccess |
-      (
-        aesObjectCreation
-            .asExpr()
-            .getExpr()
-            .(ObjectCreation)
-            .getAnArgument()
-            .getValue()
-            .stringMatches("System.Security.Cryptography.AesManaged") or
-        aesObjectCreation =
-          API::getTopLevelMember("system")
-              .getMember("security")
-              .getMember("cryptography")
-              .getMember("aes")
-              .getMember("create")
-              .asCall()
-      ) and
-      aesObjectAccess.getALocalSource() = aesObjectCreation and
-      aesObjectAccess.asExpr().getExpr() = this.getQualifier() and
+/**
+ * Holds if `name` (lowercase) is the short name of a .NET symmetric algorithm type
+ * that has a `Mode` property.
+ */
+private predicate isSymmetricAlgorithmTypeName(string name) {
+  name =
+    [
+      "aes", "aesmanaged", "aescryptoserviceprovider", "aescng",
+      "rijndael", "rijndaelmanaged",
+      "des", "descryptoserviceprovider",
+      "tripledes", "tripledescryptoserviceprovider",
+      "rc2", "rc2cryptoserviceprovider",
+      "symmetricalgorithm"
+    ]
+}
+
+/**
+ * Holds if `creation` is a data flow node that creates a symmetric algorithm object.
+ */
+private predicate isSymmetricAlgorithmCreation(DataFlow::Node creation) {
+  // New-Object "System.Security.Cryptography.Xxx" or [Xxx]::new()
+  exists(DataFlow::ObjectCreationNode objCreation, string typeName, string shortName |
+    creation = objCreation and
+    typeName = objCreation.getLowerCaseConstructedTypeName() and
+    isSymmetricAlgorithmTypeName(shortName) and
+    (
+      typeName = shortName or
+      typeName.matches("%." + shortName)
+    )
+  )
+  or
+  // [System.Security.Cryptography.Xxx]::Create()
+  exists(string typeName |
+    isSymmetricAlgorithmTypeName(typeName) and
+    creation =
+      API::getTopLevelMember("system")
+          .getMember("security")
+          .getMember("cryptography")
+          .getMember(typeName)
+          .getMember("create")
+          .asCall()
+  )
+}
+
+/**
+ * A member expression that writes to the `Mode` property of a symmetric algorithm object.
+ */
+class SymmetricAlgorithmModeProperty extends MemberExpr {
+  SymmetricAlgorithmModeProperty() {
+    exists(DataFlow::Node symAlgCreation, DataFlow::Node qualAccess |
+      isSymmetricAlgorithmCreation(symAlgCreation) and
+      qualAccess.getALocalSource() = symAlgCreation and
+      qualAccess.asExpr().getExpr() = this.getQualifier() and
       this.getLowerCaseMemberName() = "mode"
     )
   }
@@ -54,7 +86,7 @@ module Config implements DataFlow::ConfigSig {
 
   predicate isSink(DataFlow::Node sink) {
     sink.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr().getExpr() =
-      any(AesModeProperty mode).getQualifier()
+      any(SymmetricAlgorithmModeProperty mode).getQualifier()
   }
 
   predicate allowImplicitRead(DataFlow::Node n, DataFlow::ContentSet cs) {

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

@@ -1,20 +1,53 @@
 edges
+| Test.ps1:7:1:7:8 | badMode | Test.ps1:8:13:8:20 | badMode | provenance |  |
 | Test.ps1:7:1:7:8 | badMode | Test.ps1:9:20:9:27 | badMode | provenance |  |
 | Test.ps1:7:12:7:57 | ecb | Test.ps1:7:1:7:8 | badMode | provenance |  |
+| Test.ps1:8:13:8:20 | badMode | Test.ps1:8:1:8:4 | [post] aes | provenance |  |
 | Test.ps1:9:20:9:27 | badMode | Test.ps1:9:1:9:11 | [post] aesManaged | provenance |  |
+| Test.ps1:11:13:11:58 | ecb | Test.ps1:11:1:11:4 | [post] aes | provenance |  |
 | Test.ps1:12:20:12:65 | ecb | Test.ps1:12:1:12:11 | [post] aesManaged | provenance |  |
+| Test.ps1:15:13:15:17 | ecb | Test.ps1:15:1:15:4 | [post] aes | provenance |  |
 | Test.ps1:16:20:16:24 | ecb | Test.ps1:16:1:16:11 | [post] aesManaged | provenance |  |
+| Test.ps1:20:18:20:63 | ecb | Test.ps1:20:1:20:9 | [post] rijndael | provenance |  |
+| Test.ps1:23:19:23:64 | ecb | Test.ps1:23:1:23:10 | [post] tripleDes | provenance |  |
+| Test.ps1:27:16:27:61 | ecb | Test.ps1:27:1:27:7 | [post] aesCsp | provenance |  |
+| Test.ps1:31:18:31:22 | ecb | Test.ps1:31:1:31:9 | [post] aesShort | provenance |  |
+| Test.ps1:35:14:35:14 | 2 | Test.ps1:35:1:35:5 | [post] aes2 | provenance |  |
 nodes
 | Test.ps1:7:1:7:8 | badMode | semmle.label | badMode |
 | Test.ps1:7:12:7:57 | ecb | semmle.label | ecb |
+| Test.ps1:8:1:8:4 | [post] aes | semmle.label | [post] aes |
+| Test.ps1:8:13:8:20 | badMode | semmle.label | badMode |
 | Test.ps1:9:1:9:11 | [post] aesManaged | semmle.label | [post] aesManaged |
 | Test.ps1:9:20:9:27 | badMode | semmle.label | badMode |
+| Test.ps1:11:1:11:4 | [post] aes | semmle.label | [post] aes |
+| Test.ps1:11:13:11:58 | ecb | semmle.label | ecb |
 | Test.ps1:12:1:12:11 | [post] aesManaged | semmle.label | [post] aesManaged |
 | Test.ps1:12:20:12:65 | ecb | semmle.label | ecb |
+| Test.ps1:15:1:15:4 | [post] aes | semmle.label | [post] aes |
+| Test.ps1:15:13:15:17 | ecb | semmle.label | ecb |
 | Test.ps1:16:1:16:11 | [post] aesManaged | semmle.label | [post] aesManaged |
 | Test.ps1:16:20:16:24 | ecb | semmle.label | ecb |
+| Test.ps1:20:1:20:9 | [post] rijndael | semmle.label | [post] rijndael |
+| Test.ps1:20:18:20:63 | ecb | semmle.label | ecb |
+| Test.ps1:23:1:23:10 | [post] tripleDes | semmle.label | [post] tripleDes |
+| Test.ps1:23:19:23:64 | ecb | semmle.label | ecb |
+| Test.ps1:27:1:27:7 | [post] aesCsp | semmle.label | [post] aesCsp |
+| Test.ps1:27:16:27:61 | ecb | semmle.label | ecb |
+| Test.ps1:31:1:31:9 | [post] aesShort | semmle.label | [post] aesShort |
+| Test.ps1:31:18:31:22 | ecb | semmle.label | ecb |
+| Test.ps1:35:1:35:5 | [post] aes2 | semmle.label | [post] aes2 |
+| Test.ps1:35:14:35:14 | 2 | semmle.label | 2 |
 subpaths
 #select
+| Test.ps1:8:1:8:4 | [post] aes | Test.ps1:7:12:7:57 | ecb | Test.ps1:8:1:8:4 | [post] aes |  |
 | Test.ps1:9:1:9:11 | [post] aesManaged | Test.ps1:7:12:7:57 | ecb | Test.ps1:9:1:9:11 | [post] aesManaged |  |
+| Test.ps1:11:1:11:4 | [post] aes | Test.ps1:11:13:11:58 | ecb | Test.ps1:11:1:11:4 | [post] aes |  |
 | Test.ps1:12:1:12:11 | [post] aesManaged | Test.ps1:12:20:12:65 | ecb | Test.ps1:12:1:12:11 | [post] aesManaged |  |
+| Test.ps1:15:1:15:4 | [post] aes | Test.ps1:15:13:15:17 | ecb | Test.ps1:15:1:15:4 | [post] aes |  |
 | Test.ps1:16:1:16:11 | [post] aesManaged | Test.ps1:16:20:16:24 | ecb | Test.ps1:16:1:16:11 | [post] aesManaged |  |
+| Test.ps1:20:1:20:9 | [post] rijndael | Test.ps1:20:18:20:63 | ecb | Test.ps1:20:1:20:9 | [post] rijndael |  |
+| Test.ps1:23:1:23:10 | [post] tripleDes | Test.ps1:23:19:23:64 | ecb | Test.ps1:23:1:23:10 | [post] tripleDes |  |
+| Test.ps1:27:1:27:7 | [post] aesCsp | Test.ps1:27:16:27:61 | ecb | Test.ps1:27:1:27:7 | [post] aesCsp |  |
+| Test.ps1:31:1:31:9 | [post] aesShort | Test.ps1:31:18:31:22 | ecb | Test.ps1:31:1:31:9 | [post] aesShort |  |
+| Test.ps1:35:1:35:5 | [post] aes2 | Test.ps1:35:14:35:14 | 2 | Test.ps1:35:1:35:5 | [post] aes2 |  |

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

@@ -13,4 +13,29 @@ $aesManaged.Mode = [System.Security.Cryptography.CipherMode]::ECB # $ Alert
 
 # Setting weak modes directly
 $aes.Mode = "ecb"
-$aesManaged.Mode = "ecb" # $ Alert
+$aesManaged.Mode = "ecb"
+
+# Other symmetric algorithm types
+$rijndael = New-Object "System.Security.Cryptography.RijndaelManaged"
+$rijndael.Mode = [System.Security.Cryptography.CipherMode]::ECB
+
+$tripleDes = New-Object "System.Security.Cryptography.TripleDESCryptoServiceProvider"
+$tripleDes.Mode = [System.Security.Cryptography.CipherMode]::ECB
+
+# [Type]::new() constructor pattern
+$aesCsp = [System.Security.Cryptography.AesCryptoServiceProvider]::new()
+$aesCsp.Mode = [System.Security.Cryptography.CipherMode]::ECB
+
+# Partial/short type names
+$aesShort = New-Object AesManaged
+$aesShort.Mode = "ecb"
+
+# Integer cipher mode values (ECB = 2)
+$aes2 = [System.Security.Cryptography.Aes]::Create()
+$aes2.Mode = 2
+
+# Safe: CBC mode (should not be flagged)
+$aesSafe = [System.Security.Cryptography.Aes]::Create()
+$aesSafe.Mode = [System.Security.Cryptography.CipherMode]::CBC
+$aesSafe.Mode = "cbc"
+$aesSafe.Mode = 1