Skip to content

Commit 8635eaf

Browse files
chanel-ychanel-y
authored
Merge pull request #361 from microsoft/approved-cipher-mode-changes
Update ApprovedCipherMode query and tests
2 parents a3cc253 + b18703e commit 8635eaf

4 files changed

Lines changed: 151 additions & 25 deletions

File tree

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,31 @@ 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(int val |
255+
val = this.asExpr().getExpr().getValue().asInt() and
256+
cipherModeIntValue(val, modeName)
257+
)
258+
}
259+
260+
override string getName() { result = modeName }
261+
}
262+
238263
class RsaCreateKeyCreation extends AsymmetricKeyCreation, DataFlow::CallNode {
239264
int keySize;
240265

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

Lines changed: 64 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,27 +18,70 @@ 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 |
21+
/**
22+
* Holds if `name` (lowercase) is the short name of a .NET symmetric algorithm type
23+
* that has a `Mode` property.
24+
*/
25+
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+
* A data flow node that creates a symmetric algorithm object.
39+
*/
40+
abstract class SymmetricAlgorithmCreation extends DataFlow::Node { }
41+
42+
/**
43+
* A symmetric algorithm creation via `New-Object "System.Security.Cryptography.Xxx"` or `[Xxx]::new()`.
44+
*/
45+
class SymmetricAlgorithmNewObject extends SymmetricAlgorithmCreation {
46+
SymmetricAlgorithmNewObject() {
47+
exists(DataFlow::ObjectCreationNode objCreation, string typeName, string shortName |
48+
this = objCreation and
49+
typeName = objCreation.getLowerCaseConstructedTypeName() and
50+
isSymmetricAlgorithmTypeName(shortName) and
2451
(
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
52+
typeName = shortName or
53+
typeName.matches("%." + shortName)
54+
)
55+
)
56+
}
57+
}
58+
59+
/**
60+
* A symmetric algorithm creation via `[System.Security.Cryptography.Xxx]::Create()`.
61+
*/
62+
class SymmetricAlgorithmFactoryCreate extends SymmetricAlgorithmCreation {
63+
SymmetricAlgorithmFactoryCreate() {
64+
exists(string typeName |
65+
isSymmetricAlgorithmTypeName(typeName) and
66+
this =
67+
API::getTopLevelMember("system")
68+
.getMember("security")
69+
.getMember("cryptography")
70+
.getMember(typeName)
71+
.getMember("create")
72+
.asCall()
73+
)
74+
}
75+
}
76+
77+
/**
78+
* A member expression that writes to the `Mode` property of a symmetric algorithm object.
79+
*/
80+
class SymmetricAlgorithmModeProperty extends MemberExpr {
81+
SymmetricAlgorithmModeProperty() {
82+
exists(SymmetricAlgorithmCreation symAlgCreation, DataFlow::Node qualAccess |
83+
qualAccess.getALocalSource() = symAlgCreation and
84+
qualAccess.asExpr().getExpr() = this.getQualifier() and
4285
this.getLowerCaseMemberName() = "mode"
4386
)
4487
}
@@ -54,7 +97,7 @@ module Config implements DataFlow::ConfigSig {
5497

5598
predicate isSink(DataFlow::Node sink) {
5699
sink.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr().getExpr() =
57-
any(AesModeProperty mode).getQualifier()
100+
any(SymmetricAlgorithmModeProperty mode).getQualifier()
58101
}
59102

60103
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: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,37 @@ $aesManaged = New-Object "System.Security.Cryptography.AesManaged"
55

66
#Setting weak modes via CipherMode enum
77
$badMode = [System.Security.Cryptography.CipherMode]::ECB # $ Source
8-
$aes.Mode = $badMode
8+
$aes.Mode = $badMode # $ Alert
99
$aesManaged.Mode = $badMode # $ Alert
1010

11-
$aes.Mode = [System.Security.Cryptography.CipherMode]::ECB
11+
$aes.Mode = [System.Security.Cryptography.CipherMode]::ECB # $ Alert
1212
$aesManaged.Mode = [System.Security.Cryptography.CipherMode]::ECB # $ Alert
1313

1414
# Setting weak modes directly
15-
$aes.Mode = "ecb"
16-
$aesManaged.Mode = "ecb" # $ Alert
15+
$aes.Mode = "ecb" # $ Alert
16+
$aesManaged.Mode = "ecb" # $ Alert
17+
18+
# Other symmetric algorithm types
19+
$rijndael = New-Object "System.Security.Cryptography.RijndaelManaged"
20+
$rijndael.Mode = [System.Security.Cryptography.CipherMode]::ECB # $ Alert
21+
22+
$tripleDes = New-Object "System.Security.Cryptography.TripleDESCryptoServiceProvider"
23+
$tripleDes.Mode = [System.Security.Cryptography.CipherMode]::ECB # $ Alert
24+
25+
# [Type]::new() constructor pattern
26+
$aesCsp = [System.Security.Cryptography.AesCryptoServiceProvider]::new()
27+
$aesCsp.Mode = [System.Security.Cryptography.CipherMode]::ECB # $ Alert
28+
29+
# Partial/short type names
30+
$aesShort = New-Object AesManaged
31+
$aesShort.Mode = "ecb" # $ Alert
32+
33+
# Integer cipher mode values (ECB = 2)
34+
$aes2 = [System.Security.Cryptography.Aes]::Create()
35+
$aes2.Mode = 2 # $ Alert
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)