Skip to content

Commit 6648a6f

Browse files
committed
added qhelps, initial queries for cipher mode, symmetric, obsolete KDF alg
1 parent 0c0b2f2 commit 6648a6f

8 files changed

Lines changed: 332 additions & 68 deletions

File tree

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
Cipher modes determine how block ciphers process data during encryption. Some cipher modes
6+
such as Electronic Codebook (ECB) and Output Feedback (OFB) are considered weak and should
7+
not be used for encryption.
8+
</p>
9+
<p>
10+
ECB mode encrypts identical plaintext blocks into identical ciphertext blocks, which can
11+
reveal patterns in the encrypted data and leak information about the plaintext. OFB mode
12+
has weaknesses that can lead to security issues when the same initialization vector (IV)
13+
is reused.
14+
</p>
15+
</overview>
16+
<recommendation>
17+
<p>
18+
Use a secure cipher mode such as Cipher Block Chaining (CBC) with a random initialization
19+
vector (IV), or an authenticated encryption mode like Galois/Counter Mode (GCM). GCM is
20+
preferred as it provides both confidentiality and integrity.
21+
</p>
22+
</recommendation>
23+
<example>
24+
<p>
25+
The following example shows a weak cipher mode (ECB) being used for encryption:
26+
</p>
27+
<sample language="powershell">
28+
# BAD: Using ECB mode which reveals patterns in encrypted data
29+
$aes = [System.Security.Cryptography.Aes]::Create()
30+
$aes.Mode = [System.Security.Cryptography.CipherMode]::ECB
31+
</sample>
32+
<p>
33+
The following example shows a recommended approach using CBC mode with a random IV:
34+
</p>
35+
<sample language="powershell">
36+
# GOOD: Using CBC mode with a random IV
37+
$aes = [System.Security.Cryptography.Aes]::Create()
38+
$aes.Mode = [System.Security.Cryptography.CipherMode]::CBC
39+
$aes.GenerateIV()
40+
</sample>
41+
</example>
42+
<references>
43+
<li>NIST, SP 800-38A: <a href="https://csrc.nist.gov/publications/detail/sp/800-38a/final">Recommendation for Block Cipher Modes of Operation</a>.</li>
44+
<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html">Cryptographic Storage Cheat Sheet</a>.</li>
45+
<li>CWE-327: <a href="https://cwe.mitre.org/data/definitions/327.html">Use of a Broken or Risky Cryptographic Algorithm</a>.</li>
46+
</references>
47+
</qhelp>
Lines changed: 66 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,57 +1,81 @@
11
/**
2-
* @name Use of a weak cipher mode
3-
* @description Using weak cipher modes such as ECB or OFB can compromise the security of encrypted data.
4-
* @kind problem
5-
* @problem.severity error
6-
* @security-severity 7.5
7-
* @precision high
8-
* @id powershell/weak-cipher-mode
9-
* @tags security
10-
* external/cwe/cwe-327
11-
*/
2+
* @name Insecure Symmetric cipher mode
3+
* @description Use of insecure cipher mode
4+
* @problem.severity error
5+
* @kind path-problem
6+
* @security-severity 8.8
7+
* @precision high
8+
* @id powershell/microsoft/public/weak-cipher-mode
9+
* @tags correctness
10+
* security
11+
* external/cwe/cwe-327
12+
*/
1213

1314
import powershell
14-
import semmle.code.powershell.ApiGraphs
15-
import semmle.code.powershell.dataflow.TaintTracking
1615
import semmle.code.powershell.dataflow.DataFlow
16+
import semmle.code.powershell.dataflow.TaintTracking
17+
import semmle.code.powershell.ApiGraphs
18+
import WeakEncryptionFlow::PathGraph
1719

18-
class WeakCipherMode extends API::Node {
19-
WeakCipherMode() {
20-
this = API::getTopLevelMember("system").getMember("security").getMember("cryptography").getMember("ciphermode").getMember("cbc")
21-
}
22-
}
23-
24-
module WeakCipherModeConfig implements DataFlow::ConfigSig {
25-
predicate isSource(DataFlow::Node source) {
26-
exists(WeakCipherMode wcm | source = wcm.asSource())
20+
class AesCreation extends ObjectCreation {
21+
AesCreation() {
22+
this.getAnArgument().getValue().stringMatches("System.Security.Cryptography.AesManaged")
23+
}
2724
}
2825

29-
predicate isSink(DataFlow::Node sink) { any() }
30-
26+
class AesModeProperty extends MemberExpr {
27+
AesModeProperty() {
28+
exists(DataFlow::ObjectCreationNode aesObjectCreation, DataFlow::Node aesObjectAccess |
29+
(
30+
aesObjectCreation.asExpr().getExpr().(ObjectCreation).getAnArgument().getValue().stringMatches("System.Security.Cryptography.AesManaged") or
31+
aesObjectCreation.(DataFlow::CallNode)= API::getTopLevelMember("system").getMember("security").getMember("cryptography").getMember("aes").getMember("create").asCall()
32+
)
33+
and
34+
aesObjectAccess.getALocalSource() = aesObjectCreation and
35+
aesObjectAccess.asExpr().getExpr() = this.getQualifier() and
36+
this.getLowerCaseMemberName() = "mode"
37+
)
38+
}
3139
}
3240

33-
module CommandInjectionFlow = TaintTracking::Global<WeakCipherModeConfig>;
34-
35-
36-
37-
//dataflow from WeakCipherMode to Mode property of System.Security.Cryptography.Aes object!
38-
39-
from DataFlow::Node mode
40-
where mode = API::getTopLevelMember("system")
41+
class WeakAesMode extends DataFlow::Node {
42+
WeakAesMode() {
43+
exists(API::Node node, string modeValue |
44+
node =
45+
API::getTopLevelMember("system")
4146
.getMember("security")
4247
.getMember("cryptography")
43-
.getMember("aes")
44-
.getMember("mode")
45-
.asSink()
46-
// select mode, "mode member of aes"
48+
.getMember("ciphermode")
49+
.getMember(modeValue) and
50+
modeValue = ["ecb", "ofb", "cfb", "ctr", "obc"] and
51+
this = node.asSource()
52+
) or
53+
exists(StringConstExpr s |
54+
s.getValueString().toLowerCase() = ["ecb", "ofb", "cfb", "ctr", "obc"] and
55+
this.asExpr().getExpr() = s
56+
)
57+
}
58+
}
59+
60+
module Config implements DataFlow::ConfigSig {
61+
predicate isSource(DataFlow::Node source) { source instanceof WeakAesMode }
4762

48-
from API::Node item
49-
select item, "node"
63+
predicate isSink(DataFlow::Node sink) {
64+
sink.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr().getExpr() =
65+
any(AesModeProperty mode).getQualifier()
66+
}
5067

68+
predicate allowImplicitRead(DataFlow::Node n, DataFlow::ContentSet cs) {
69+
isSink(n) and
70+
exists(DataFlow::Content::FieldContent fc |
71+
cs.isSingleton(fc) and
72+
fc.getLowerCaseName() = "mode"
73+
)
74+
}
75+
}
5176

52-
// from API::Node sink
53-
// where sink = API::getTopLevelMember("system").getMember("security").getMember("cryptography").getMember("ciphermode").getMember("cbc")
54-
// select sink, sink.asSource()
77+
module WeakEncryptionFlow = TaintTracking::Global<Config>;
5578

56-
// from InvokeEncryptModeArgument a
57-
// select a, "Use of weak cipher mode in encryption."
79+
from WeakEncryptionFlow::PathNode source, WeakEncryptionFlow::PathNode sink
80+
where WeakEncryptionFlow::flowPath(source, sink)
81+
select sink.getNode(), source, sink, ""
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
Key Derivation Functions (KDFs) are used to derive cryptographic keys from passwords or
6+
other secret values. Using obsolete or weak KDF algorithms can compromise password security.
7+
</p>
8+
<p>
9+
The <code>PasswordDeriveBytes</code> class implements the PBKDF1 algorithm, which is
10+
considered obsolete and insecure. PBKDF1 has known weaknesses and is limited to producing
11+
keys that are at most 160 bits in length.
12+
</p>
13+
</overview>
14+
<recommendation>
15+
<p>
16+
Use the <code>Rfc2898DeriveBytes</code> class which implements the PBKDF2 algorithm with
17+
a SHA-2 hash function (such as SHA-256 or SHA-512). Additionally, use a high iteration
18+
count (at least 100,000) to increase resistance against brute-force attacks.
19+
</p>
20+
</recommendation>
21+
<example>
22+
<p>
23+
The following example shows the use of an obsolete KDF algorithm:
24+
</p>
25+
<sample language="powershell">
26+
# BAD: Using PasswordDeriveBytes which implements PBKDF1
27+
$kdf = New-Object System.Security.Cryptography.PasswordDeriveBytes($password, $salt)
28+
$key = $kdf.CryptDeriveKey("AES", "SHA256", 256, $iv)
29+
</sample>
30+
<p>
31+
The following example shows the recommended approach using PBKDF2:
32+
</p>
33+
<sample language="powershell">
34+
# GOOD: Using Rfc2898DeriveBytes with SHA-256 and high iteration count
35+
$kdf = New-Object System.Security.Cryptography.Rfc2898DeriveBytes($password, $salt, 100000, [System.Security.Cryptography.HashAlgorithmName]::SHA256)
36+
$key = $kdf.GetBytes(32)
37+
</sample>
38+
</example>
39+
<references>
40+
<li>NIST, SP 800-132: <a href="https://csrc.nist.gov/publications/detail/sp/800-132/final">Recommendation for Password-Based Key Derivation</a>.</li>
41+
<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html">Password Storage Cheat Sheet</a>.</li>
42+
<li>CWE-327: <a href="https://cwe.mitre.org/data/definitions/327.html">Use of a Broken or Risky Cryptographic Algorithm</a>.</li>
43+
<li>CWE-328: <a href="https://cwe.mitre.org/data/definitions/328.html">Use of Weak Hash</a>.</li>
44+
</references>
45+
</qhelp>
Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
/**
22
* @name Use of obsolete Key Derivation Function (KDF) algorithm
3-
* @description Using obsolete or weak KDF algorithms like PasswordDeriveBytes (PBKDF1)
4-
* instead of secure alternatives like Rfc2898DeriveBytes (PBKDF2) can
5-
* compromise password security.
3+
* @description Do not use obsolete or weak KDF algorithms like PasswordDeriveBytes (PBKDF1)
4+
* instead of secure alternatives like Rfc2898DeriveBytes (PBKDF2)
65
* @kind problem
76
* @problem.severity error
87
* @security-severity 7.5
@@ -25,34 +24,21 @@ class CryptDeriveKeyCall extends DataFlow::CallNode {
2524
this = API::getTopLevelMember("system")
2625
.getMember("security")
2726
.getMember("cryptography")
28-
.getMember("passwordderivebytes")
27+
.getMember("passwordderivebytes").getInstance()
2928
.getMember("cryptderivekey")
3029
.asCall()
3130
or
3231
this = API::getTopLevelMember("system")
3332
.getMember("security")
3433
.getMember("cryptography")
35-
.getMember("rfc2898derivebytes")
34+
.getMember("rfc2898derivebytes").getInstance()
3635
.getMember("cryptderivekey")
3736
.asCall()
3837
}
3938
}
4039

41-
// from DataFlow::CallNode cn
42-
// where
43-
// cn instanceof CryptDeriveKeyCall
44-
// select cn, "Use of obsolete Crypto API. Consider using Rfc2898DeriveBytes (PBKDF2) or a more modern alternative like Argon2."
40+
from DataFlow::CallNode cn
41+
where
42+
cn instanceof CryptDeriveKeyCall
43+
select cn, "Use of obsolete Crypto API. Password-based key derivation should use the PBKDF2 algorithm with SHA-2 hashing"
4544

46-
// from DataFlow::CallNode cn
47-
// select cn, "cn"
48-
// from CryptDeriveKeyCall cn
49-
// select cn, "Use of obsolete KDF algorithm PasswordDeriveBytes (PBKDF1). Consider using Rfc2898DeriveBytes (PBKDF2) or a more modern alternative like Argon2."
50-
51-
from DataFlow::CallNode apiNode
52-
where
53-
apiNode = API::getTopLevelMember("system")
54-
.getMember("security")
55-
.getMember("cryptography")
56-
.getMember("passwordderivebytes")
57-
.getMember("cryptderivekey").asCall()
58-
select apiNode, "node"
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
Cryptographic hash functions are used to verify data integrity and for password storage.
6+
Using weak or broken cryptographic hash algorithms can compromise security.
7+
</p>
8+
<p>
9+
The MD5 and SHA-1 hash algorithms are considered cryptographically weak and should not
10+
be used for security-sensitive purposes. MD5 has known collision vulnerabilities, and
11+
SHA-1 has been demonstrated to be vulnerable to practical collision attacks.
12+
</p>
13+
</overview>
14+
<recommendation>
15+
<p>
16+
Use a strong cryptographic hash algorithm from the SHA-2 family, such as SHA-256, SHA-384,
17+
or SHA-512. For password hashing, consider using a specialized password hashing function
18+
like PBKDF2, bcrypt, or Argon2.
19+
</p>
20+
</recommendation>
21+
<example>
22+
<p>
23+
The following example shows the use of weak hash algorithms:
24+
</p>
25+
<sample language="powershell">
26+
# BAD: Using MD5 which is cryptographically broken
27+
$md5 = [System.Security.Cryptography.MD5]::Create()
28+
$hash = $md5.ComputeHash($data)
29+
30+
# BAD: Using SHA1 which has known collision vulnerabilities
31+
$sha1 = [System.Security.Cryptography.SHA1]::Create()
32+
$hash = $sha1.ComputeHash($data)
33+
</sample>
34+
<p>
35+
The following example shows the recommended approach using SHA-256:
36+
</p>
37+
<sample language="powershell">
38+
# GOOD: Using SHA-256 which is a strong cryptographic hash
39+
$sha256 = [System.Security.Cryptography.SHA256]::Create()
40+
$hash = $sha256.ComputeHash($data)
41+
</sample>
42+
</example>
43+
<references>
44+
<li>NIST, SP 800-131A: <a href="https://csrc.nist.gov/publications/detail/sp/800-131a/rev-2/final">Transitioning the Use of Cryptographic Algorithms and Key Lengths</a>.</li>
45+
<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html">Password Storage Cheat Sheet</a>.</li>
46+
<li>CWE-327: <a href="https://cwe.mitre.org/data/definitions/327.html">Use of a Broken or Risky Cryptographic Algorithm</a>.</li>
47+
<li>CWE-328: <a href="https://cwe.mitre.org/data/definitions/328.html">Use of Weak Hash</a>.</li>
48+
</references>
49+
</qhelp>

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ class WeakHashAlgorithmObjectCreation extends DataFlow::ObjectCreationNode {
2525
}
2626
}
2727

28-
class WeakHashAlgorithmObjectCreate extends DataFlow::CallNode {
29-
WeakHashAlgorithmObjectCreate() {
28+
class WeakHashAlgorithmCreateCall extends DataFlow::CallNode {
29+
WeakHashAlgorithmCreateCall() {
3030
this = API::getTopLevelMember("system").getMember("security").getMember("cryptography").getMember("md5").getMember("create").asCall() or
3131
this = API::getTopLevelMember("system").getMember("security").getMember("cryptography").getMember("sha1").getMember("create").asCall()
3232
}
@@ -58,6 +58,6 @@ class CreateFromNameSink extends DataFlow::CallNode {
5858
from DataFlow::Node sink
5959
where sink instanceof ComputeHashSink or
6060
sink instanceof WeakHashAlgorithmObjectCreation or
61-
sink instanceof WeakHashAlgorithmObjectCreate or
61+
sink instanceof WeakHashAlgorithmCreateCall or
6262
sink instanceof CreateFromNameSink
63-
select sink, "Use of weak cryptographic hash algorithm."
63+
select sink, "Use of weak cryptographic hash algorithm."
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
Symmetric encryption algorithms are used to protect data confidentiality. Using broken
6+
or weak cryptographic algorithms can compromise the security of encrypted data.
7+
</p>
8+
<p>
9+
The DES, Triple DES (3DES), RC2, and Rijndael algorithms are considered weak or deprecated:
10+
</p>
11+
<ul>
12+
<li>DES has a 56-bit key which is too short and can be broken with modern hardware.</li>
13+
<li>Triple DES (3DES) is deprecated due to its small block size (64 bits) which makes it vulnerable to birthday attacks.</li>
14+
<li>RC2 has known weaknesses and uses variable key sizes that may not provide adequate security.</li>
15+
<li>Rijndael is the original name for AES but should be accessed through the AES class for standardized behavior.</li>
16+
</ul>
17+
</overview>
18+
<recommendation>
19+
<p>
20+
Use the AES (Advanced Encryption Standard) algorithm with a key size of 128, 192, or 256 bits.
21+
AES is the current standard for symmetric encryption and is considered secure when used correctly.
22+
</p>
23+
</recommendation>
24+
<example>
25+
<p>
26+
The following examples show the use of weak symmetric algorithms:
27+
</p>
28+
<sample language="powershell">
29+
# BAD: Using DES which has a 56-bit key
30+
$des = [System.Security.Cryptography.DES]::Create()
31+
32+
# BAD: Using Triple DES which is deprecated
33+
$tdes = [System.Security.Cryptography.TripleDES]::Create()
34+
35+
# BAD: Using RC2 which has known weaknesses
36+
$rc2 = [System.Security.Cryptography.RC2]::Create()
37+
</sample>
38+
<p>
39+
The following example shows the recommended approach using AES:
40+
</p>
41+
<sample language="powershell">
42+
# GOOD: Using AES which is the current standard
43+
$aes = [System.Security.Cryptography.Aes]::Create()
44+
$aes.KeySize = 256
45+
$aes.GenerateKey()
46+
$aes.GenerateIV()
47+
</sample>
48+
</example>
49+
<references>
50+
<li>NIST, FIPS 197: <a href="https://csrc.nist.gov/publications/detail/fips/197/final">Advanced Encryption Standard (AES)</a>.</li>
51+
<li>NIST, SP 800-131A: <a href="https://csrc.nist.gov/publications/detail/sp/800-131a/rev-2/final">Transitioning the Use of Cryptographic Algorithms and Key Lengths</a>.</li>
52+
<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html#rule---use-strong-approved-authenticated-encryption">Rule - Use strong approved cryptographic algorithms</a>.</li>
53+
<li>CWE-327: <a href="https://cwe.mitre.org/data/definitions/327.html">Use of a Broken or Risky Cryptographic Algorithm</a>.</li>
54+
</references>
55+
</qhelp>

0 commit comments

Comments
 (0)