From 6e238a761342afd4773446b958c98f7c4b5bb4da Mon Sep 17 00:00:00 2001 From: Chanel Young Date: Wed, 20 May 2026 12:05:25 -0700 Subject: [PATCH 1/2] Update ApprovedCipherMode query and tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../cryptography/CryptographyModule.qll | 26 +++++++ .../security/cwe-327/ApprovedCipherMode.ql | 76 +++++++++++++------ .../ApprovedCipherMode.expected | 33 ++++++++ .../cwe-327/ApprovedCipherMode/Test.ps1 | 27 ++++++- 4 files changed, 139 insertions(+), 23 deletions(-) diff --git a/powershell/ql/lib/semmle/code/powershell/security/cryptography/CryptographyModule.qll b/powershell/ql/lib/semmle/code/powershell/security/cryptography/CryptographyModule.qll index 2bd114cb9b49..808ceea335ff 100644 --- a/powershell/ql/lib/semmle/code/powershell/security/cryptography/CryptographyModule.qll +++ b/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; diff --git a/powershell/ql/src/queries/security/cwe-327/ApprovedCipherMode.ql b/powershell/ql/src/queries/security/cwe-327/ApprovedCipherMode.ql index e387ae85e11a..663c82a44fee 100644 --- a/powershell/ql/src/queries/security/cwe-327/ApprovedCipherMode.ql +++ b/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) { diff --git a/powershell/ql/test/query-tests/security/cwe-327/ApprovedCipherMode/ApprovedCipherMode.expected b/powershell/ql/test/query-tests/security/cwe-327/ApprovedCipherMode/ApprovedCipherMode.expected index 8b031c69dadc..3a925af5cd39 100644 --- a/powershell/ql/test/query-tests/security/cwe-327/ApprovedCipherMode/ApprovedCipherMode.expected +++ b/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 | | diff --git a/powershell/ql/test/query-tests/security/cwe-327/ApprovedCipherMode/Test.ps1 b/powershell/ql/test/query-tests/security/cwe-327/ApprovedCipherMode/Test.ps1 index 99e6ac5d1d7a..4533273056a9 100644 --- a/powershell/ql/test/query-tests/security/cwe-327/ApprovedCipherMode/Test.ps1 +++ b/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 \ No newline at end of file +$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 From a5e53797b31cc67428227912a244710701e61414 Mon Sep 17 00:00:00 2001 From: Chanel Young Date: Thu, 21 May 2026 12:25:28 -0700 Subject: [PATCH 2/2] predicate -> abstract class, updated inline expectations --- .../security/cwe-327/ApprovedCipherMode.ql | 63 +++++++++++-------- .../cwe-327/ApprovedCipherMode/Test.ps1 | 18 +++--- 2 files changed, 46 insertions(+), 35 deletions(-) diff --git a/powershell/ql/src/queries/security/cwe-327/ApprovedCipherMode.ql b/powershell/ql/src/queries/security/cwe-327/ApprovedCipherMode.ql index 663c82a44fee..aa549024b055 100644 --- a/powershell/ql/src/queries/security/cwe-327/ApprovedCipherMode.ql +++ b/powershell/ql/src/queries/security/cwe-327/ApprovedCipherMode.ql @@ -22,7 +22,7 @@ import WeakEncryptionFlow::PathGraph * Holds if `name` (lowercase) is the short name of a .NET symmetric algorithm type * that has a `Mode` property. */ -private predicate isSymmetricAlgorithmTypeName(string name) { +predicate isSymmetricAlgorithmTypeName(string name) { name = [ "aes", "aesmanaged", "aescryptoserviceprovider", "aescng", @@ -35,31 +35,43 @@ private predicate isSymmetricAlgorithmTypeName(string name) { } /** - * Holds if `creation` is a data flow node that creates a symmetric algorithm object. + * 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) +abstract class SymmetricAlgorithmCreation extends DataFlow::Node { } + +/** + * A symmetric algorithm creation via `New-Object "System.Security.Cryptography.Xxx"` or `[Xxx]::new()`. + */ +class SymmetricAlgorithmNewObject extends SymmetricAlgorithmCreation { + SymmetricAlgorithmNewObject() { + exists(DataFlow::ObjectCreationNode objCreation, string typeName, string shortName | + this = objCreation and + typeName = objCreation.getLowerCaseConstructedTypeName() and + isSymmetricAlgorithmTypeName(shortName) and + ( + typeName = shortName or + typeName.matches("%." + shortName) + ) + ) + } +} + +/** + * A symmetric algorithm creation via `[System.Security.Cryptography.Xxx]::Create()`. + */ +class SymmetricAlgorithmFactoryCreate extends SymmetricAlgorithmCreation { + SymmetricAlgorithmFactoryCreate() { + exists(string typeName | + isSymmetricAlgorithmTypeName(typeName) and + this = + API::getTopLevelMember("system") + .getMember("security") + .getMember("cryptography") + .getMember(typeName) + .getMember("create") + .asCall() ) - ) - 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() - ) + } } /** @@ -67,8 +79,7 @@ private predicate isSymmetricAlgorithmCreation(DataFlow::Node creation) { */ class SymmetricAlgorithmModeProperty extends MemberExpr { SymmetricAlgorithmModeProperty() { - exists(DataFlow::Node symAlgCreation, DataFlow::Node qualAccess | - isSymmetricAlgorithmCreation(symAlgCreation) and + exists(SymmetricAlgorithmCreation symAlgCreation, DataFlow::Node qualAccess | qualAccess.getALocalSource() = symAlgCreation and qualAccess.asExpr().getExpr() = this.getQualifier() and this.getLowerCaseMemberName() = "mode" diff --git a/powershell/ql/test/query-tests/security/cwe-327/ApprovedCipherMode/Test.ps1 b/powershell/ql/test/query-tests/security/cwe-327/ApprovedCipherMode/Test.ps1 index 4533273056a9..22e9c817c18f 100644 --- a/powershell/ql/test/query-tests/security/cwe-327/ApprovedCipherMode/Test.ps1 +++ b/powershell/ql/test/query-tests/security/cwe-327/ApprovedCipherMode/Test.ps1 @@ -5,34 +5,34 @@ $aesManaged = New-Object "System.Security.Cryptography.AesManaged" #Setting weak modes via CipherMode enum $badMode = [System.Security.Cryptography.CipherMode]::ECB # $ Source -$aes.Mode = $badMode +$aes.Mode = $badMode # $ Alert $aesManaged.Mode = $badMode # $ Alert -$aes.Mode = [System.Security.Cryptography.CipherMode]::ECB +$aes.Mode = [System.Security.Cryptography.CipherMode]::ECB # $ Alert $aesManaged.Mode = [System.Security.Cryptography.CipherMode]::ECB # $ Alert # Setting weak modes directly -$aes.Mode = "ecb" -$aesManaged.Mode = "ecb" +$aes.Mode = "ecb" # $ Alert +$aesManaged.Mode = "ecb" # $ Alert # Other symmetric algorithm types $rijndael = New-Object "System.Security.Cryptography.RijndaelManaged" -$rijndael.Mode = [System.Security.Cryptography.CipherMode]::ECB +$rijndael.Mode = [System.Security.Cryptography.CipherMode]::ECB # $ Alert $tripleDes = New-Object "System.Security.Cryptography.TripleDESCryptoServiceProvider" -$tripleDes.Mode = [System.Security.Cryptography.CipherMode]::ECB +$tripleDes.Mode = [System.Security.Cryptography.CipherMode]::ECB # $ Alert # [Type]::new() constructor pattern $aesCsp = [System.Security.Cryptography.AesCryptoServiceProvider]::new() -$aesCsp.Mode = [System.Security.Cryptography.CipherMode]::ECB +$aesCsp.Mode = [System.Security.Cryptography.CipherMode]::ECB # $ Alert # Partial/short type names $aesShort = New-Object AesManaged -$aesShort.Mode = "ecb" +$aesShort.Mode = "ecb" # $ Alert # Integer cipher mode values (ECB = 2) $aes2 = [System.Security.Cryptography.Aes]::Create() -$aes2.Mode = 2 +$aes2.Mode = 2 # $ Alert # Safe: CBC mode (should not be flagged) $aesSafe = [System.Security.Cryptography.Aes]::Create()