Skip to content

Commit 1211b9d

Browse files
authored
Merge pull request #379 from microsoft/weak-kdf-powershell
PS: Weak kdf query
2 parents fd99a4a + 4618044 commit 1211b9d

8 files changed

Lines changed: 425 additions & 0 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
When deriving cryptographic keys from passwords using <code>Rfc2898DeriveBytes</code> (PBKDF2),
8+
both the iteration count and hash algorithm must be configured securely.
9+
An insufficient iteration count or a weak hash algorithm makes the derived key
10+
vulnerable to brute-force attacks.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
Always specify at least 100,000 iterations and use SHA-256 or a stronger hash algorithm
17+
(SHA-384, SHA-512) when creating an <code>Rfc2898DeriveBytes</code> instance or calling the
18+
static <code>Pbkdf2</code> method.
19+
</p>
20+
</recommendation>
21+
22+
<example>
23+
<p>The following example shows insecure usage with default settings:</p>
24+
<sample src="examples/WeakKDFConfigurationBad.ps1" />
25+
<p>The following example shows secure usage with adequate iterations and a strong hash:</p>
26+
<sample src="examples/WeakKDFConfigurationGood.ps1" />
27+
</example>
28+
29+
<references>
30+
<li>
31+
OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html">Password Storage Cheat Sheet</a>
32+
</li>
33+
<li>
34+
Microsoft: <a href="https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.rfc2898derivebytes">Rfc2898DeriveBytes Class</a>
35+
</li>
36+
<li>
37+
CWE-327: <a href="https://cwe.mitre.org/data/definitions/327.html">Use of a Broken or Risky Cryptographic Algorithm</a>
38+
</li>
39+
</references>
40+
</qhelp>
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* @name Weak key derivation function configuration
3+
* @description Rfc2898DeriveBytes (PBKDF2) should use at least 100,000 iterations
4+
* and a hash algorithm of SHA-256 or stronger to resist brute-force attacks.
5+
* @kind problem
6+
* @problem.severity error
7+
* @security-severity 7.5
8+
* @precision high
9+
* @id powershell/weak-kdf-configuration
10+
* @tags security
11+
* external/cwe/cwe-327
12+
* external/cwe/cwe-328
13+
* cryptography
14+
*/
15+
16+
import powershell
17+
import WeakKDFConfiguration
18+
19+
from WeakKdfConfig config
20+
select config, config.getMessage()
Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
/**
2+
* Provides classes and predicates for reasoning about weak key derivation
3+
* function (KDF) configurations using `Rfc2898DeriveBytes` (PBKDF2).
4+
*/
5+
6+
import powershell
7+
import semmle.code.powershell.ApiGraphs
8+
import semmle.code.powershell.dataflow.DataFlow
9+
10+
/** Gets the minimum recommended PBKDF2 iteration count. */
11+
int minIterationCount() { result = 100000 }
12+
13+
/** Gets the `System.Security.Cryptography` namespace. */
14+
private API::Node cryptographyNamespace() {
15+
result =
16+
API::getTopLevelMember("system").getMember("security").getMember("cryptography")
17+
}
18+
19+
/** Gets the `System.Security.Cryptography.Rfc2898DeriveBytes` type. */
20+
private API::Node rfc2898DeriveBytesType() {
21+
result = cryptographyNamespace().getMember("rfc2898derivebytes")
22+
}
23+
24+
/**
25+
* An instantiation of Rfc2898DeriveBytes via New-Object or [Type]::new().
26+
*/
27+
class Rfc2898DeriveBytesCreation extends DataFlow::CallNode {
28+
Rfc2898DeriveBytesCreation() { this = rfc2898DeriveBytesType().getInstance().asSource() }
29+
30+
private DataFlow::Node getNewObjectArgumentList() {
31+
this.getExprNode().getExpr() instanceof DotNetObjectCreation and
32+
(
33+
result = this.getNamedArgument("argumentlist")
34+
or
35+
not this.hasNamedArgument("argumentlist") and result = this.getPositionalArgument(1)
36+
)
37+
}
38+
39+
private DataFlow::Node getNewObjectArgument(int index) {
40+
exists(ArrayLiteral args |
41+
args = this.getNewObjectArgumentList().asExpr().getExpr() and
42+
result.asExpr().getExpr() = args.getExpr(index)
43+
)
44+
or
45+
exists(ParenExpr paren, ArrayLiteral args |
46+
paren = this.getNewObjectArgumentList().asExpr().getExpr() and
47+
args = paren.getExpr() and
48+
result.asExpr().getExpr() = args.getExpr(index)
49+
)
50+
}
51+
52+
private predicate hasKnownNewObjectArgumentList() {
53+
this.getNewObjectArgumentList().asExpr().getExpr() instanceof ArrayLiteral
54+
or
55+
this.getNewObjectArgumentList().asExpr().getExpr().(ParenExpr).getExpr() instanceof ArrayLiteral
56+
}
57+
58+
private DataFlow::Node getConstructorArgument(int index) {
59+
this.getExprNode().getExpr() instanceof NewObjectCreation and
60+
result = this.getPositionalArgument(index)
61+
or
62+
result = this.getNewObjectArgument(index)
63+
}
64+
65+
/** Gets the iteration count argument (position 2, 0-indexed), if any. */
66+
DataFlow::Node getIterationCountArg() { result = this.getConstructorArgument(2) }
67+
68+
/** Gets the hash algorithm argument (position 3, 0-indexed), if any. */
69+
DataFlow::Node getHashAlgorithmArg() { result = this.getConstructorArgument(3) }
70+
71+
/** Holds if the constructor is known to omit the iteration count argument. */
72+
predicate hasDefaultIterationCount() {
73+
not this.getExprNode().getExpr() instanceof DotNetObjectCreation and
74+
not exists(this.getIterationCountArg())
75+
or
76+
this.hasKnownNewObjectArgumentList() and not exists(this.getNewObjectArgument(2))
77+
}
78+
79+
/** Holds if the constructor is known to omit the hash algorithm argument. */
80+
predicate hasDefaultHashAlgorithm() {
81+
not this.getExprNode().getExpr() instanceof DotNetObjectCreation and
82+
not exists(this.getHashAlgorithmArg())
83+
or
84+
this.hasKnownNewObjectArgumentList() and not exists(this.getNewObjectArgument(3))
85+
}
86+
}
87+
88+
/**
89+
* A call to the static Rfc2898DeriveBytes.Pbkdf2 method (.NET 6+).
90+
*/
91+
class Pbkdf2StaticCall extends DataFlow::CallNode {
92+
Pbkdf2StaticCall() { this = rfc2898DeriveBytesType().getMember("pbkdf2").asCall() }
93+
94+
/** Gets the iteration count argument (position 2, 0-indexed). */
95+
DataFlow::Node getIterationCountArg() { result = this.getPositionalArgument(2) }
96+
97+
/** Gets the hash algorithm argument (position 3, 0-indexed). */
98+
DataFlow::Node getHashAlgorithmArg() { result = this.getPositionalArgument(3) }
99+
}
100+
101+
/**
102+
* Holds if `node` is an integer literal less than the minimum iteration count.
103+
*/
104+
predicate isLowIterationValue(DataFlow::Node node, int value) {
105+
value = node.asExpr().getExpr().getValue().asInt() and
106+
value < minIterationCount()
107+
}
108+
109+
/**
110+
* Holds if `node` references a weak hash algorithm (MD5 or SHA1).
111+
*/
112+
predicate isWeakHashAlgorithm(DataFlow::Node node, string name) {
113+
// [HashAlgorithmName]::MD5 or [HashAlgorithmName]::SHA1
114+
node = cryptographyNamespace().getMember("hashalgorithmname").getMember(name).asSource() and
115+
name = ["md5", "sha1"]
116+
or
117+
// String literal "MD5" or "SHA1"
118+
exists(string s |
119+
s = node.asExpr().getExpr().getValue().asString().toLowerCase() and
120+
s = ["md5", "sha1"] and
121+
name = s
122+
)
123+
}
124+
125+
/**
126+
* A weak key derivation function configuration that should be reported.
127+
*/
128+
abstract class WeakKdfConfig extends DataFlow::CallNode {
129+
abstract string getMessage();
130+
}
131+
132+
/**
133+
* Rfc2898DeriveBytes created without specifying an iteration count.
134+
*/
135+
class DefaultIterationCountConfig extends WeakKdfConfig, Rfc2898DeriveBytesCreation {
136+
DefaultIterationCountConfig() { this.hasDefaultIterationCount() }
137+
138+
override string getMessage() {
139+
result =
140+
"Rfc2898DeriveBytes uses default iteration count of 1000. Specify at least " +
141+
minIterationCount().toString() + " iterations."
142+
}
143+
}
144+
145+
/**
146+
* Rfc2898DeriveBytes created with a low iteration count.
147+
*/
148+
class LowIterationCountConfig extends WeakKdfConfig, Rfc2898DeriveBytesCreation {
149+
LowIterationCountConfig() {
150+
isLowIterationValue(this.getIterationCountArg(), _)
151+
}
152+
153+
override string getMessage() {
154+
exists(int value |
155+
isLowIterationValue(this.getIterationCountArg(), value) and
156+
result =
157+
"Rfc2898DeriveBytes uses iteration count of " + value.toString() +
158+
", which is below the minimum of " + minIterationCount().toString() + "."
159+
)
160+
}
161+
}
162+
163+
/**
164+
* Rfc2898DeriveBytes created without specifying a hash algorithm (defaults to SHA1).
165+
*/
166+
class DefaultHashAlgorithmConfig extends WeakKdfConfig, Rfc2898DeriveBytesCreation {
167+
DefaultHashAlgorithmConfig() { this.hasDefaultHashAlgorithm() }
168+
169+
override string getMessage() {
170+
result = "Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger."
171+
}
172+
}
173+
174+
/**
175+
* Rfc2898DeriveBytes created with a weak hash algorithm.
176+
*/
177+
class WeakHashAlgorithmConfig extends WeakKdfConfig, Rfc2898DeriveBytesCreation {
178+
WeakHashAlgorithmConfig() {
179+
isWeakHashAlgorithm(this.getHashAlgorithmArg(), _)
180+
}
181+
182+
override string getMessage() {
183+
exists(string name |
184+
isWeakHashAlgorithm(this.getHashAlgorithmArg(), name) and
185+
result =
186+
"Rfc2898DeriveBytes uses weak hash algorithm " + name.toUpperCase() +
187+
". Use SHA-256 or stronger."
188+
)
189+
}
190+
}
191+
192+
/**
193+
* Rfc2898DeriveBytes.Pbkdf2 called with a low iteration count.
194+
*/
195+
class Pbkdf2LowIterationCountConfig extends WeakKdfConfig, Pbkdf2StaticCall {
196+
Pbkdf2LowIterationCountConfig() {
197+
isLowIterationValue(this.getIterationCountArg(), _)
198+
}
199+
200+
override string getMessage() {
201+
exists(int value |
202+
isLowIterationValue(this.getIterationCountArg(), value) and
203+
result =
204+
"Rfc2898DeriveBytes.Pbkdf2 uses iteration count of " + value.toString() +
205+
", which is below the minimum of " + minIterationCount().toString() + "."
206+
)
207+
}
208+
}
209+
210+
/**
211+
* Rfc2898DeriveBytes.Pbkdf2 called with a weak hash algorithm.
212+
*/
213+
class Pbkdf2WeakHashAlgorithmConfig extends WeakKdfConfig, Pbkdf2StaticCall {
214+
Pbkdf2WeakHashAlgorithmConfig() {
215+
isWeakHashAlgorithm(this.getHashAlgorithmArg(), _)
216+
}
217+
218+
override string getMessage() {
219+
exists(string name |
220+
isWeakHashAlgorithm(this.getHashAlgorithmArg(), name) and
221+
result =
222+
"Rfc2898DeriveBytes.Pbkdf2 uses weak hash algorithm " + name.toUpperCase() +
223+
". Use SHA-256 or stronger."
224+
)
225+
}
226+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# BAD: Default iteration count (1000) and default hash algorithm (SHA1)
2+
$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt)
3+
4+
# BAD: Low iteration count
5+
$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 1000)
6+
7+
# BAD: Weak hash algorithm
8+
$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 100000, [System.Security.Cryptography.HashAlgorithmName]::SHA1)
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# GOOD: 100,000+ iterations with SHA-256
2+
$kdf = [System.Security.Cryptography.Rfc2898DeriveBytes]::new($password, $salt, 600000, [System.Security.Cryptography.HashAlgorithmName]::SHA256)
3+
4+
# GOOD: Static Pbkdf2 with strong configuration
5+
$key = [System.Security.Cryptography.Rfc2898DeriveBytes]::Pbkdf2($password, $salt, 600000, "SHA256", 32)
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
| WeakKDFConfiguration.ps1:7:8:7:79 | Call to new | Rfc2898DeriveBytes uses default iteration count of 1000. Specify at least 100000 iterations. |
2+
| WeakKDFConfiguration.ps1:7:8:7:79 | Call to new | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
3+
| WeakKDFConfiguration.ps1:11:8:11:85 | Call to new | Rfc2898DeriveBytes uses iteration count of 1000, which is below the minimum of 100000. |
4+
| WeakKDFConfiguration.ps1:11:8:11:85 | Call to new | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
5+
| WeakKDFConfiguration.ps1:14:8:14:86 | Call to new | Rfc2898DeriveBytes uses iteration count of 10000, which is below the minimum of 100000. |
6+
| WeakKDFConfiguration.ps1:14:8:14:86 | Call to new | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
7+
| WeakKDFConfiguration.ps1:17:8:17:86 | Call to new | Rfc2898DeriveBytes uses iteration count of 50000, which is below the minimum of 100000. |
8+
| WeakKDFConfiguration.ps1:17:8:17:86 | Call to new | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
9+
| WeakKDFConfiguration.ps1:21:8:21:87 | Call to new | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
10+
| WeakKDFConfiguration.ps1:25:8:25:143 | Call to new | Rfc2898DeriveBytes uses weak hash algorithm SHA1. Use SHA-256 or stronger. |
11+
| WeakKDFConfiguration.ps1:28:8:28:142 | Call to new | Rfc2898DeriveBytes uses weak hash algorithm MD5. Use SHA-256 or stronger. |
12+
| WeakKDFConfiguration.ps1:32:8:32:89 | Call to new-object | Rfc2898DeriveBytes uses iteration count of 5000, which is below the minimum of 100000. |
13+
| WeakKDFConfiguration.ps1:32:8:32:89 | Call to new-object | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
14+
| WeakKDFConfiguration.ps1:36:8:36:83 | Call to new-object | Rfc2898DeriveBytes uses default iteration count of 1000. Specify at least 100000 iterations. |
15+
| WeakKDFConfiguration.ps1:36:8:36:83 | Call to new-object | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
16+
| WeakKDFConfiguration.ps1:39:8:41:1 | Call to new-object | Rfc2898DeriveBytes uses iteration count of 5000, which is below the minimum of 100000. |
17+
| WeakKDFConfiguration.ps1:39:8:41:1 | Call to new-object | Rfc2898DeriveBytes uses weak hash algorithm SHA1. Use SHA-256 or stronger. |
18+
| WeakKDFConfiguration.ps1:42:8:44:1 | Call to new-object | Rfc2898DeriveBytes uses iteration count of 5000, which is below the minimum of 100000. |
19+
| WeakKDFConfiguration.ps1:42:8:44:1 | Call to new-object | Rfc2898DeriveBytes uses weak hash algorithm SHA1. Use SHA-256 or stronger. |
20+
| WeakKDFConfiguration.ps1:48:8:48:102 | Call to pbkdf2 | Rfc2898DeriveBytes.Pbkdf2 uses iteration count of 1000, which is below the minimum of 100000. |
21+
| WeakKDFConfiguration.ps1:52:8:52:102 | Call to pbkdf2 | Rfc2898DeriveBytes.Pbkdf2 uses weak hash algorithm SHA1. Use SHA-256 or stronger. |
22+
| WeakKDFConfiguration.ps1:55:8:55:84 | Call to new | Rfc2898DeriveBytes uses iteration count of 500, which is below the minimum of 100000. |
23+
| WeakKDFConfiguration.ps1:55:8:55:84 | Call to new | Rfc2898DeriveBytes uses the default hash algorithm SHA1. Specify SHA-256 or stronger. |
24+
| WeakKDFConfiguration.ps1:58:8:58:140 | Call to new | Rfc2898DeriveBytes uses iteration count of 500, which is below the minimum of 100000. |
25+
| WeakKDFConfiguration.ps1:58:8:58:140 | Call to new | Rfc2898DeriveBytes uses weak hash algorithm SHA1. Use SHA-256 or stronger. |

0 commit comments

Comments
 (0)