Skip to content

Commit 3cd00a9

Browse files
committed
initial query, insecure random to sensitive sinks
1 parent 90860cb commit 3cd00a9

8 files changed

Lines changed: 407 additions & 56 deletions

File tree

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for reasoning about
3+
* insecure-randomness vulnerabilities, as well as extension points for
4+
* adding your own.
5+
*/
6+
7+
private import semmle.code.powershell.dataflow.DataFlow
8+
import semmle.code.powershell.ApiGraphs
9+
10+
module InsecureRandomness {
11+
/**
12+
* A data flow source for insecure-randomness vulnerabilities.
13+
*/
14+
abstract class Source extends DataFlow::Node { }
15+
16+
/**
17+
* A data flow sink for insecure-randomness vulnerabilities.
18+
*/
19+
abstract class Sink extends DataFlow::Node {
20+
/** Gets a human-readable description of this sink. */
21+
abstract string getSinkDescription();
22+
}
23+
24+
/**
25+
* A sanitizer for insecure-randomness vulnerabilities.
26+
*/
27+
abstract class Sanitizer extends DataFlow::Node { }
28+
29+
/** A call to the `Get-Random` cmdlet. */
30+
class GetRandomSource extends Source instanceof DataFlow::CallNode {
31+
GetRandomSource() { this.matchesName("Get-Random") }
32+
}
33+
34+
/** An instantiation of `System.Random` via `New-Object`. */
35+
class SystemRandomObjectCreation extends Source instanceof DataFlow::ObjectCreationNode {
36+
SystemRandomObjectCreation() {
37+
this.asExpr()
38+
.getExpr()
39+
.(ObjectCreation)
40+
.getAnArgument()
41+
.getValue()
42+
.stringMatches("System.Random")
43+
}
44+
}
45+
46+
/** A call to `[System.Random]::new()` via the API graph. */
47+
class SystemRandomNewCall extends Source instanceof DataFlow::CallNode {
48+
SystemRandomNewCall() {
49+
this = API::getTopLevelMember("system").getMember("random").getMember("new").asCall()
50+
}
51+
}
52+
53+
/** Assignment to .Key or .IV on an object. */
54+
class CryptoKeyIvAssignmentSink extends Sink {
55+
CryptoKeyIvAssignmentSink() {
56+
exists(AssignStmt a, MemberExprWriteAccess m |
57+
m = a.getLeftHandSide() and
58+
m.getLowerCaseMemberName() in ["key", "iv"] and
59+
this.asExpr().getExpr() = a.getRightHandSide()
60+
)
61+
}
62+
63+
override string getSinkDescription() { result = "a cryptographic key or IV" }
64+
}
65+
66+
/** Argument to a cryptographic constructor (HMAC key or KDF salt). */
67+
class CryptoConstructorSink extends Sink {
68+
CryptoConstructorSink() {
69+
exists(DataFlow::ObjectCreationNode oc |
70+
(
71+
oc.getLowerCaseConstructedTypeName().matches("%hmac%")
72+
or
73+
oc.getLowerCaseConstructedTypeName().matches("%rfc2898derivebytes%")
74+
) and
75+
this = oc.getAnArgument() and
76+
not this = oc.getConstructedTypeNode()
77+
)
78+
}
79+
80+
override string getSinkDescription() { result = "a cryptographic operation" }
81+
}
82+
83+
/** First positional argument to ConvertTo-SecureString. */
84+
class SecureStringSink extends Sink {
85+
SecureStringSink() {
86+
exists(DataFlow::CallNode call |
87+
call.matchesName("ConvertTo-SecureString") and
88+
this = call.getPositionalArgument(0)
89+
)
90+
}
91+
92+
override string getSinkDescription() { result = "a secure string conversion" }
93+
}
94+
95+
/** Value used in HTTP headers passed to Invoke-RestMethod or Invoke-WebRequest. */
96+
class HttpHeaderValueSink extends Sink {
97+
HttpHeaderValueSink() {
98+
exists(DataFlow::CallNode call, HashTableExpr ht |
99+
(call.matchesName("Invoke-RestMethod") or call.matchesName("Invoke-WebRequest")) and
100+
call.getNamedArgument("headers").asExpr().getExpr() = ht and
101+
this.asExpr().getExpr() = ht.getAValue()
102+
)
103+
}
104+
105+
override string getSinkDescription() { result = "an HTTP header" }
106+
}
107+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/**
2+
* Provides a taint tracking configuration for reasoning about
3+
* insecure-randomness vulnerabilities (CWE-338).
4+
*
5+
* Note, for performance reasons: only import this file if
6+
* `InsecureRandomnessFlow` is needed, otherwise
7+
* `InsecureRandomnessCustomizations` should be imported instead.
8+
*/
9+
10+
import powershell
11+
import semmle.code.powershell.dataflow.TaintTracking
12+
import InsecureRandomnessCustomizations::InsecureRandomness
13+
import semmle.code.powershell.dataflow.DataFlow
14+
15+
private module Config implements DataFlow::ConfigSig {
16+
predicate isSource(DataFlow::Node source) { source instanceof Source }
17+
18+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
19+
20+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
21+
22+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
23+
// When .NextBytes() is called on a System.Random instance,
24+
// taint flows from the qualifier to the argument (the byte array is filled with random bytes).
25+
exists(DataFlow::CallNode call |
26+
call.matchesName("NextBytes") and
27+
node1 = call.getQualifier() and
28+
node2.(DataFlow::PostUpdateNode).getPreUpdateNode() = call.getArgument(0)
29+
)
30+
}
31+
}
32+
33+
/**
34+
* Taint-tracking for reasoning about insecure-randomness vulnerabilities.
35+
*/
36+
module InsecureRandomnessFlow = TaintTracking::Global<Config>;
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>Using non-cryptographic random number generators for security-sensitive operations can compromise security. `Get-Random` and `System.Random` are not suitable for generating cryptographic keys, security tokens, or other security-critical random values.</p>
6+
</overview>
7+
8+
<recommendation>
9+
<p>Use a cryptographically secure random number generator such as `[System.Security.Cryptography.RandomNumberGenerator]` instead.</p>
10+
</recommendation>
11+
12+
<example>
13+
<p>Insecure random:</p>
14+
<sample src="examples/InsecureRandomness/InsecureRandomnessBad.ps1" />
15+
16+
<p>Secure alternative:</p>
17+
<sample src="examples/InsecureRandomness/InsecureRandomnessGood.ps1" />
18+
</example>
19+
20+
<references>
21+
<li><a href="https://cwe.mitre.org/data/definitions/338.html">CWE-338: Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG)</a>.</li>
22+
<li><a href="https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.randomnumbergenerator">System.Security.Cryptography.RandomNumberGenerator</a>.</li>
23+
</references>
24+
25+
</qhelp>
Lines changed: 10 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,56 +1,23 @@
11
/**
2-
* @name Use of insecure random number generator
2+
* @name Use of insecure random number generator in security-sensitive context
33
* @description Using non-cryptographic random number generators such as Get-Random or System.Random
44
* for security-sensitive operations can compromise security.
5-
* @kind problem
5+
* @kind path-problem
66
* @problem.severity error
77
* @security-severity 7.5
88
* @precision high
9-
* @id powershell/microsoft/security/insecure-random
9+
* @id powershell/insecure-randomness
1010
* @tags security
1111
* external/cwe/cwe-330
1212
* external/cwe/cwe-338
1313
*/
1414

1515
import powershell
16-
import semmle.code.powershell.ApiGraphs
17-
import semmle.code.powershell.dataflow.DataFlow
16+
import semmle.code.powershell.security.InsecureRandomnessQuery
17+
import InsecureRandomnessFlow::PathGraph
1818

19-
/** A call to the `Get-Random` cmdlet. */
20-
class GetRandomCall extends DataFlow::CallNode {
21-
GetRandomCall() { this.matchesName("Get-Random") }
22-
}
23-
24-
/** An instantiation of `System.Random` via `New-Object`. */
25-
class SystemRandomObjectCreation extends DataFlow::ObjectCreationNode {
26-
SystemRandomObjectCreation() {
27-
this.asExpr()
28-
.getExpr()
29-
.(ObjectCreation)
30-
.getAnArgument()
31-
.getValue()
32-
.stringMatches("System.Random")
33-
}
34-
}
35-
36-
/** A call to `[System.Random]::new()` via the API graph. */
37-
class SystemRandomNewCall extends DataFlow::CallNode {
38-
SystemRandomNewCall() {
39-
this = API::getTopLevelMember("system").getMember("random").getMember("new").asCall()
40-
}
41-
}
42-
43-
from DataFlow::Node node, string msg
44-
where
45-
node instanceof GetRandomCall and
46-
msg =
47-
"Use of insecure random number generator 'Get-Random'. Use [System.Security.Cryptography.RandomNumberGenerator] instead."
48-
or
49-
node instanceof SystemRandomObjectCreation and
50-
msg =
51-
"Use of insecure random number generator 'System.Random'. Use [System.Security.Cryptography.RandomNumberGenerator] instead."
52-
or
53-
node instanceof SystemRandomNewCall and
54-
msg =
55-
"Use of insecure random number generator 'System.Random'. Use [System.Security.Cryptography.RandomNumberGenerator] instead."
56-
select node, msg
19+
from InsecureRandomnessFlow::PathNode source, InsecureRandomnessFlow::PathNode sink
20+
where InsecureRandomnessFlow::flowPath(source, sink)
21+
select sink.getNode(), source, sink,
22+
"Insecure random value flows to " + sink.getNode().(Sink).getSinkDescription() + " from $@.",
23+
source.getNode(), "this insecure random source"
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# BAD: weak RNG output flows into security-sensitive .NET crypto sinks.
2+
3+
# 1) Weak bytes flow into AES .Key and .IV properties
4+
$weak = New-Object System.Random
5+
$keyBytes = New-Object byte[] 32
6+
$ivBytes = New-Object byte[] 16
7+
$weak.NextBytes($keyBytes)
8+
$weak.NextBytes($ivBytes)
9+
$aes = [System.Security.Cryptography.Aes]::Create()
10+
$aes.Key = $keyBytes # BAD: predictable key
11+
$aes.IV = $ivBytes # BAD: predictable IV
12+
13+
# 2) Weak bytes flow into HMAC constructor (signing key)
14+
$hmacKeyBytes = New-Object byte[] 64
15+
$weak.NextBytes($hmacKeyBytes)
16+
$hmac = New-Object System.Security.Cryptography.HMACSHA256(,$hmacKeyBytes) # BAD: predictable HMAC key
17+
18+
# 3) Weak bytes used as salt for password-based key derivation
19+
$saltBytes = New-Object byte[] 8
20+
$weak.NextBytes($saltBytes)
21+
$kdf = New-Object System.Security.Cryptography.Rfc2898DeriveBytes("password", $saltBytes) # BAD: predictable salt
22+
23+
# 4) Get-Random result flows into ConvertTo-SecureString → PSCredential
24+
$tempPwd = (Get-Random -Maximum 999999999).ToString()
25+
$securePwd = ConvertTo-SecureString $tempPwd -AsPlainText -Force
26+
$cred = New-Object System.Management.Automation.PSCredential("admin", $securePwd) # BAD: predictable credential
27+
28+
# 5) Weak random flows into HTTP Authorization header
29+
$tokenValue = Get-Random -Maximum 999999999
30+
Invoke-RestMethod -Uri "https://api.example.com/data" `
31+
-Headers @{ Authorization = "Bearer $tokenValue" } # BAD: predictable bearer token
32+
33+
# 6) Weak random used as RNGCryptoServiceProvider substitute for key bytes
34+
$rng2 = [System.Random]::new()
35+
$aesKey2 = New-Object byte[] 32
36+
$rng2.NextBytes($aesKey2)
37+
[Security.Cryptography.RNGCryptoServiceProvider]::Create().GetBytes($aesKey2) | Out-Null
38+
# The above line re-fills the buffer, but the initial weak fill might persist
39+
# if code logic is wrong. True BAD pattern: relying solely on System.Random:
40+
$aes2 = [System.Security.Cryptography.Aes]::Create()
41+
$rng2.NextBytes($aesKey2) # BAD: final value comes from weak RNG
42+
$aes2.Key = $aesKey2 # BAD: predictable key assigned to cipher
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# GOOD: cryptographic RNG output flows into security-sensitive .NET crypto sinks.
2+
3+
# 1) CSPRNG bytes flow into AES .Key and .IV properties
4+
$keyBytes = New-Object byte[] 32
5+
$ivBytes = New-Object byte[] 16
6+
[System.Security.Cryptography.RandomNumberGenerator]::Fill($keyBytes)
7+
[System.Security.Cryptography.RandomNumberGenerator]::Fill($ivBytes)
8+
$aes = [System.Security.Cryptography.Aes]::Create()
9+
$aes.Key = $keyBytes # GOOD: cryptographically random key
10+
$aes.IV = $ivBytes # GOOD: cryptographically random IV
11+
12+
# 2) CSPRNG bytes flow into HMAC constructor (signing key)
13+
$hmacKeyBytes = New-Object byte[] 64
14+
[System.Security.Cryptography.RandomNumberGenerator]::Fill($hmacKeyBytes)
15+
$hmac = New-Object System.Security.Cryptography.HMACSHA256(,$hmacKeyBytes) # GOOD
16+
17+
# 3) CSPRNG bytes used as salt for password-based key derivation
18+
$saltBytes = New-Object byte[] 16
19+
[System.Security.Cryptography.RandomNumberGenerator]::Fill($saltBytes)
20+
$kdf = New-Object System.Security.Cryptography.Rfc2898DeriveBytes("password", $saltBytes) # GOOD
21+
22+
# 4) CSPRNG bytes flow into ConvertTo-SecureString → PSCredential
23+
$pwdBytes = New-Object byte[] 32
24+
[System.Security.Cryptography.RandomNumberGenerator]::Fill($pwdBytes)
25+
$tempPwd = [Convert]::ToBase64String($pwdBytes)
26+
$securePwd = ConvertTo-SecureString $tempPwd -AsPlainText -Force
27+
$cred = New-Object System.Management.Automation.PSCredential("admin", $securePwd) # GOOD
28+
29+
# 5) CSPRNG bytes flow into HTTP Authorization header
30+
$tokenBytes = New-Object byte[] 32
31+
[System.Security.Cryptography.RandomNumberGenerator]::Fill($tokenBytes)
32+
$bearerToken = [Convert]::ToBase64String($tokenBytes)
33+
Invoke-RestMethod -Uri "https://api.example.com/data" `
34+
-Headers @{ Authorization = "Bearer $bearerToken" } # GOOD
35+
36+
# 6) CSPRNG via RNGCryptoServiceProvider fills key bytes for cipher
37+
$rng = [Security.Cryptography.RNGCryptoServiceProvider]::Create()
38+
$aesKey2 = New-Object byte[] 32
39+
$rng.GetBytes($aesKey2) # GOOD: filled by CSPRNG
40+
$aes2 = [System.Security.Cryptography.Aes]::Create()
41+
$aes2.Key = $aesKey2 # GOOD: unpredictable key

0 commit comments

Comments
 (0)