Skip to content

Commit 6e238a7

Browse files
chanel-yCopilot
chanel-y
and
Copilot
committed
Update ApprovedCipherMode query and tests
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a3cc253 commit 6e238a7

4 files changed

Lines changed: 139 additions & 23 deletions

File tree

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,32 @@ class CipherBlockModeEnum extends BlockMode {
235235
override string getName() { result = modeName }
236236
}
237237

238+
private predicate cipherModeIntValue(int val, string name) {
239+
val = 1 and name = "cbc"
240+
or
241+
val = 2 and name = "ecb"
242+
or
243+
val = 3 and name = "ofb"
244+
or
245+
val = 4 and name = "cfb"
246+
or
247+
val = 5 and name = "cts"
248+
}
249+
250+
class CipherBlockModeIntConst extends BlockMode {
251+
string modeName;
252+
253+
CipherBlockModeIntConst() {
254+
exists(ConstExpr c, int val |
255+
c = this.asExpr().getExpr() and
256+
val = c.getValueString().toInt() and
257+
cipherModeIntValue(val, modeName)
258+
)
259+
}
260+
261+
override string getName() { result = modeName }
262+
}
263+
238264
class RsaCreateKeyCreation extends AsymmetricKeyCreation, DataFlow::CallNode {
239265
int keySize;
240266

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

Lines changed: 54 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,27 +18,59 @@ import semmle.code.powershell.ApiGraphs
1818
import semmle.code.powershell.security.cryptography.Concepts
1919
import WeakEncryptionFlow::PathGraph
2020

21-
class AesModeProperty extends MemberExpr {
22-
AesModeProperty() {
23-
exists(DataFlow::ObjectCreationNode aesObjectCreation, DataFlow::Node aesObjectAccess |
24-
(
25-
aesObjectCreation
26-
.asExpr()
27-
.getExpr()
28-
.(ObjectCreation)
29-
.getAnArgument()
30-
.getValue()
31-
.stringMatches("System.Security.Cryptography.AesManaged") or
32-
aesObjectCreation =
33-
API::getTopLevelMember("system")
34-
.getMember("security")
35-
.getMember("cryptography")
36-
.getMember("aes")
37-
.getMember("create")
38-
.asCall()
39-
) and
40-
aesObjectAccess.getALocalSource() = aesObjectCreation and
41-
aesObjectAccess.asExpr().getExpr() = this.getQualifier() and
21+
/**
22+
* Holds if `name` (lowercase) is the short name of a .NET symmetric algorithm type
23+
* that has a `Mode` property.
24+
*/
25+
private predicate isSymmetricAlgorithmTypeName(string name) {
26+
name =
27+
[
28+
"aes", "aesmanaged", "aescryptoserviceprovider", "aescng",
29+
"rijndael", "rijndaelmanaged",
30+
"des", "descryptoserviceprovider",
31+
"tripledes", "tripledescryptoserviceprovider",
32+
"rc2", "rc2cryptoserviceprovider",
33+
"symmetricalgorithm"
34+
]
35+
}
36+
37+
/**
38+
* Holds if `creation` is a data flow node that creates a symmetric algorithm object.
39+
*/
40+
private predicate isSymmetricAlgorithmCreation(DataFlow::Node creation) {
41+
// New-Object "System.Security.Cryptography.Xxx" or [Xxx]::new()
42+
exists(DataFlow::ObjectCreationNode objCreation, string typeName, string shortName |
43+
creation = objCreation and
44+
typeName = objCreation.getLowerCaseConstructedTypeName() and
45+
isSymmetricAlgorithmTypeName(shortName) and
46+
(
47+
typeName = shortName or
48+
typeName.matches("%." + shortName)
49+
)
50+
)
51+
or
52+
// [System.Security.Cryptography.Xxx]::Create()
53+
exists(string typeName |
54+
isSymmetricAlgorithmTypeName(typeName) and
55+
creation =
56+
API::getTopLevelMember("system")
57+
.getMember("security")
58+
.getMember("cryptography")
59+
.getMember(typeName)
60+
.getMember("create")
61+
.asCall()
62+
)
63+
}
64+
65+
/**
66+
* A member expression that writes to the `Mode` property of a symmetric algorithm object.
67+
*/
68+
class SymmetricAlgorithmModeProperty extends MemberExpr {
69+
SymmetricAlgorithmModeProperty() {
70+
exists(DataFlow::Node symAlgCreation, DataFlow::Node qualAccess |
71+
isSymmetricAlgorithmCreation(symAlgCreation) and
72+
qualAccess.getALocalSource() = symAlgCreation and
73+
qualAccess.asExpr().getExpr() = this.getQualifier() and
4274
this.getLowerCaseMemberName() = "mode"
4375
)
4476
}
@@ -54,7 +86,7 @@ module Config implements DataFlow::ConfigSig {
5486

5587
predicate isSink(DataFlow::Node sink) {
5688
sink.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr().getExpr() =
57-
any(AesModeProperty mode).getQualifier()
89+
any(SymmetricAlgorithmModeProperty mode).getQualifier()
5890
}
5991

6092
predicate allowImplicitRead(DataFlow::Node n, DataFlow::ContentSet cs) {

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,53 @@
11
edges
2+
| Test.ps1:7:1:7:8 | badMode | Test.ps1:8:13:8:20 | badMode | provenance | |
23
| Test.ps1:7:1:7:8 | badMode | Test.ps1:9:20:9:27 | badMode | provenance | |
34
| Test.ps1:7:12:7:57 | ecb | Test.ps1:7:1:7:8 | badMode | provenance | |
5+
| Test.ps1:8:13:8:20 | badMode | Test.ps1:8:1:8:4 | [post] aes | provenance | |
46
| Test.ps1:9:20:9:27 | badMode | Test.ps1:9:1:9:11 | [post] aesManaged | provenance | |
7+
| Test.ps1:11:13:11:58 | ecb | Test.ps1:11:1:11:4 | [post] aes | provenance | |
58
| Test.ps1:12:20:12:65 | ecb | Test.ps1:12:1:12:11 | [post] aesManaged | provenance | |
9+
| Test.ps1:15:13:15:17 | ecb | Test.ps1:15:1:15:4 | [post] aes | provenance | |
610
| Test.ps1:16:20:16:24 | ecb | Test.ps1:16:1:16:11 | [post] aesManaged | provenance | |
11+
| Test.ps1:20:18:20:63 | ecb | Test.ps1:20:1:20:9 | [post] rijndael | provenance | |
12+
| Test.ps1:23:19:23:64 | ecb | Test.ps1:23:1:23:10 | [post] tripleDes | provenance | |
13+
| Test.ps1:27:16:27:61 | ecb | Test.ps1:27:1:27:7 | [post] aesCsp | provenance | |
14+
| Test.ps1:31:18:31:22 | ecb | Test.ps1:31:1:31:9 | [post] aesShort | provenance | |
15+
| Test.ps1:35:14:35:14 | 2 | Test.ps1:35:1:35:5 | [post] aes2 | provenance | |
716
nodes
817
| Test.ps1:7:1:7:8 | badMode | semmle.label | badMode |
918
| Test.ps1:7:12:7:57 | ecb | semmle.label | ecb |
19+
| Test.ps1:8:1:8:4 | [post] aes | semmle.label | [post] aes |
20+
| Test.ps1:8:13:8:20 | badMode | semmle.label | badMode |
1021
| Test.ps1:9:1:9:11 | [post] aesManaged | semmle.label | [post] aesManaged |
1122
| Test.ps1:9:20:9:27 | badMode | semmle.label | badMode |
23+
| Test.ps1:11:1:11:4 | [post] aes | semmle.label | [post] aes |
24+
| Test.ps1:11:13:11:58 | ecb | semmle.label | ecb |
1225
| Test.ps1:12:1:12:11 | [post] aesManaged | semmle.label | [post] aesManaged |
1326
| Test.ps1:12:20:12:65 | ecb | semmle.label | ecb |
27+
| Test.ps1:15:1:15:4 | [post] aes | semmle.label | [post] aes |
28+
| Test.ps1:15:13:15:17 | ecb | semmle.label | ecb |
1429
| Test.ps1:16:1:16:11 | [post] aesManaged | semmle.label | [post] aesManaged |
1530
| Test.ps1:16:20:16:24 | ecb | semmle.label | ecb |
31+
| Test.ps1:20:1:20:9 | [post] rijndael | semmle.label | [post] rijndael |
32+
| Test.ps1:20:18:20:63 | ecb | semmle.label | ecb |
33+
| Test.ps1:23:1:23:10 | [post] tripleDes | semmle.label | [post] tripleDes |
34+
| Test.ps1:23:19:23:64 | ecb | semmle.label | ecb |
35+
| Test.ps1:27:1:27:7 | [post] aesCsp | semmle.label | [post] aesCsp |
36+
| Test.ps1:27:16:27:61 | ecb | semmle.label | ecb |
37+
| Test.ps1:31:1:31:9 | [post] aesShort | semmle.label | [post] aesShort |
38+
| Test.ps1:31:18:31:22 | ecb | semmle.label | ecb |
39+
| Test.ps1:35:1:35:5 | [post] aes2 | semmle.label | [post] aes2 |
40+
| Test.ps1:35:14:35:14 | 2 | semmle.label | 2 |
1641
subpaths
1742
#select
43+
| Test.ps1:8:1:8:4 | [post] aes | Test.ps1:7:12:7:57 | ecb | Test.ps1:8:1:8:4 | [post] aes | |
1844
| Test.ps1:9:1:9:11 | [post] aesManaged | Test.ps1:7:12:7:57 | ecb | Test.ps1:9:1:9:11 | [post] aesManaged | |
45+
| Test.ps1:11:1:11:4 | [post] aes | Test.ps1:11:13:11:58 | ecb | Test.ps1:11:1:11:4 | [post] aes | |
1946
| Test.ps1:12:1:12:11 | [post] aesManaged | Test.ps1:12:20:12:65 | ecb | Test.ps1:12:1:12:11 | [post] aesManaged | |
47+
| Test.ps1:15:1:15:4 | [post] aes | Test.ps1:15:13:15:17 | ecb | Test.ps1:15:1:15:4 | [post] aes | |
2048
| Test.ps1:16:1:16:11 | [post] aesManaged | Test.ps1:16:20:16:24 | ecb | Test.ps1:16:1:16:11 | [post] aesManaged | |
49+
| Test.ps1:20:1:20:9 | [post] rijndael | Test.ps1:20:18:20:63 | ecb | Test.ps1:20:1:20:9 | [post] rijndael | |
50+
| Test.ps1:23:1:23:10 | [post] tripleDes | Test.ps1:23:19:23:64 | ecb | Test.ps1:23:1:23:10 | [post] tripleDes | |
51+
| Test.ps1:27:1:27:7 | [post] aesCsp | Test.ps1:27:16:27:61 | ecb | Test.ps1:27:1:27:7 | [post] aesCsp | |
52+
| Test.ps1:31:1:31:9 | [post] aesShort | Test.ps1:31:18:31:22 | ecb | Test.ps1:31:1:31:9 | [post] aesShort | |
53+
| 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‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,29 @@ $aesManaged.Mode = [System.Security.Cryptography.CipherMode]::ECB # $ Alert
1313

1414
# Setting weak modes directly
1515
$aes.Mode = "ecb"
16-
$aesManaged.Mode = "ecb" # $ Alert
16+
$aesManaged.Mode = "ecb"
17+
18+
# Other symmetric algorithm types
19+
$rijndael = New-Object "System.Security.Cryptography.RijndaelManaged"
20+
$rijndael.Mode = [System.Security.Cryptography.CipherMode]::ECB
21+
22+
$tripleDes = New-Object "System.Security.Cryptography.TripleDESCryptoServiceProvider"
23+
$tripleDes.Mode = [System.Security.Cryptography.CipherMode]::ECB
24+
25+
# [Type]::new() constructor pattern
26+
$aesCsp = [System.Security.Cryptography.AesCryptoServiceProvider]::new()
27+
$aesCsp.Mode = [System.Security.Cryptography.CipherMode]::ECB
28+
29+
# Partial/short type names
30+
$aesShort = New-Object AesManaged
31+
$aesShort.Mode = "ecb"
32+
33+
# Integer cipher mode values (ECB = 2)
34+
$aes2 = [System.Security.Cryptography.Aes]::Create()
35+
$aes2.Mode = 2
36+
37+
# Safe: CBC mode (should not be flagged)
38+
$aesSafe = [System.Security.Cryptography.Aes]::Create()
39+
$aesSafe.Mode = [System.Security.Cryptography.CipherMode]::CBC
40+
$aesSafe.Mode = "cbc"
41+
$aesSafe.Mode = 1

0 commit comments

Comments
 (0)