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..aa549024b055 100644 --- a/powershell/ql/src/queries/security/cwe-327/ApprovedCipherMode.ql +++ b/powershell/ql/src/queries/security/cwe-327/ApprovedCipherMode.ql @@ -18,27 +18,70 @@ 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 | +/** + * Holds if `name` (lowercase) is the short name of a .NET symmetric algorithm type + * that has a `Mode` property. + */ +predicate isSymmetricAlgorithmTypeName(string name) { + name = + [ + "aes", "aesmanaged", "aescryptoserviceprovider", "aescng", + "rijndael", "rijndaelmanaged", + "des", "descryptoserviceprovider", + "tripledes", "tripledescryptoserviceprovider", + "rc2", "rc2cryptoserviceprovider", + "symmetricalgorithm" + ] +} + +/** + * A data flow node that creates a symmetric algorithm object. + */ +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 ( - 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 + 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() + ) + } +} + +/** + * A member expression that writes to the `Mode` property of a symmetric algorithm object. + */ +class SymmetricAlgorithmModeProperty extends MemberExpr { + SymmetricAlgorithmModeProperty() { + exists(SymmetricAlgorithmCreation symAlgCreation, DataFlow::Node qualAccess | + qualAccess.getALocalSource() = symAlgCreation and + qualAccess.asExpr().getExpr() = this.getQualifier() and this.getLowerCaseMemberName() = "mode" ) } @@ -54,7 +97,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..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,12 +5,37 @@ $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" # $ Alert \ No newline at end of file +$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 # $ Alert + +$tripleDes = New-Object "System.Security.Cryptography.TripleDESCryptoServiceProvider" +$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 # $ Alert + +# Partial/short type names +$aesShort = New-Object AesManaged +$aesShort.Mode = "ecb" # $ Alert + +# Integer cipher mode values (ECB = 2) +$aes2 = [System.Security.Cryptography.Aes]::Create() +$aes2.Mode = 2 # $ Alert + +# 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