Skip to content

Commit acb1923

Browse files
committed
cpp: add insecure randomness query with RNG security models
Add the cpp/insecure-randomness query (CWE-330/338) that flags cryptographically insecure random numbers used as security-sensitive values (keys, IVs, nonces). - shared quantum: add isCryptographicallySecure() to Crypto::RandomNumberGenerationInstance (defaults to none). - cpp quantum lib: model C stdlib, POSIX/BSD, Windows CNG, and C++ <random> generators via the new randomNumberGeneratorModel extensible predicate; classify OpenSSL RAND_bytes/RAND_priv_bytes as secure and RAND_pseudo_bytes as insecure. - MaD: populate randomNumberGeneratorModel with the generator rows.
1 parent b756a08 commit acb1923

15 files changed

Lines changed: 789 additions & 1 deletion

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added models of random number generation from the C standard library, POSIX/BSD, the Windows CryptoAPI/CNG, and the C++ `<random>` engines as instances of the `Crypto::RandomNumberGenerationInstance` concept, each classified as cryptographically secure or insecure. The set of generators is defined as data through the new `randomNumberGeneratorModel` extensible predicate, so it can be extended by data-extension packs. The OpenSSL `RAND_pseudo_bytes` function is now classified as insecure, while `RAND_bytes` and `RAND_priv_bytes` are classified as secure.

cpp/ql/lib/experimental/quantum/Language.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,4 @@ private class ConstantDataSource extends Crypto::GenericConstantSourceInstance i
110110
}
111111

112112
import OpenSSL.OpenSSL
113+
import Standard.Random

cpp/ql/lib/experimental/quantum/OpenSSL/Random.qll

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,18 @@ private import semmle.code.cpp.dataflow.new.DataFlow
66
class OpenSslRandomNumberGeneratorInstance extends Crypto::RandomNumberGenerationInstance instanceof Call
77
{
88
OpenSslRandomNumberGeneratorInstance() {
9-
this.(Call).getTarget().getName() in ["RAND_bytes", "RAND_pseudo_bytes"]
9+
this.(Call).getTarget().getName() in ["RAND_bytes", "RAND_priv_bytes", "RAND_pseudo_bytes"]
1010
}
1111

1212
override Crypto::DataFlowNode getOutputNode() {
1313
result.asDefiningArgument() = this.(Call).getArgument(0)
1414
}
1515

1616
override string getGeneratorName() { result = this.(Call).getTarget().getName() }
17+
18+
override predicate isCryptographicallySecure() {
19+
// `RAND_pseudo_bytes` is deprecated and does not guarantee cryptographically
20+
// secure output, so it is deliberately excluded here.
21+
this.(Call).getTarget().getName() in ["RAND_bytes", "RAND_priv_bytes"]
22+
}
1723
}
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/**
2+
* Models random number generation from the C standard library, POSIX/BSD, the
3+
* Windows CryptoAPI/CNG, and the C++ `<random>` engines, as instances of the
4+
* shared quantum `Crypto::RandomNumberGenerationInstance` concept.
5+
*
6+
* The set of modelled generators is defined as data through the
7+
* `randomNumberGeneratorModel` extensible predicate, so that downstream packs can
8+
* register additional generators without editing this library. Each row records
9+
* whether the generator is cryptographically secure; insecure generators (e.g.
10+
* `rand`, `std::mt19937`) leave `isCryptographicallySecure()` at its default of
11+
* holding for no generator.
12+
*
13+
* Only functions that *produce* random output are modelled here. Seeding
14+
* functions such as `srand`, `srandom`, `srand48`, and `seed48` produce no output
15+
* artifact and are therefore out of scope for this concept.
16+
*/
17+
18+
import cpp
19+
private import experimental.quantum.Language
20+
21+
/**
22+
* Holds if a call to the function `name` is a random number generator.
23+
*
24+
* `namespace` and `type` identify the function: when `type` is empty, `name` is a
25+
* global or `std` free function (e.g. `rand`); otherwise `name` is a member
26+
* function of the class (template) whose unqualified name is `type` (e.g.
27+
* `operator()` of `std::mersenne_twister_engine`).
28+
*
29+
* `output` is the index of the argument into which the random bytes are written,
30+
* or the empty string if the random value is the return value.
31+
*
32+
* `secure` holds if the generator is cryptographically secure.
33+
*/
34+
extensible predicate randomNumberGeneratorModel(
35+
string namespace, string type, string name, string output, boolean secure
36+
);
37+
38+
/**
39+
* Holds if `c` is a call to a modelled random number generator named
40+
* `generatorName`, writing its output as described by `output` (see
41+
* `randomNumberGeneratorModel`), where `secure` holds if it is cryptographically
42+
* secure.
43+
*/
44+
private predicate randomNumberGeneratorCall(
45+
Call c, string generatorName, string output, boolean secure
46+
) {
47+
exists(string namespace, string type, string name, Function f |
48+
randomNumberGeneratorModel(namespace, type, name, output, secure) and
49+
f = c.getTarget()
50+
|
51+
// A global or `std` free function, e.g. `rand` or `std::rand`.
52+
type = "" and
53+
f.hasGlobalOrStdName(name) and
54+
generatorName = name
55+
or
56+
// A member function of a class (template), e.g. `std::mt19937::operator()`.
57+
type != "" and
58+
f.getName() = name and
59+
f.getDeclaringType().getSimpleName() = type and
60+
(if namespace = "" then generatorName = type else generatorName = namespace + "::" + type)
61+
)
62+
}
63+
64+
/**
65+
* A call to a random number generator modelled through the `randomNumberGeneratorModel`
66+
* extensible predicate.
67+
*/
68+
class ModeledRandomNumberGeneratorInstance extends Crypto::RandomNumberGenerationInstance instanceof Call
69+
{
70+
string generatorName;
71+
string output;
72+
boolean secure;
73+
74+
ModeledRandomNumberGeneratorInstance() {
75+
randomNumberGeneratorCall(this, generatorName, output, secure)
76+
}
77+
78+
override Crypto::DataFlowNode getOutputNode() {
79+
output = "" and result.asExpr() = this
80+
or
81+
output != "" and result.asDefiningArgument() = super.getArgument(output.toInt())
82+
}
83+
84+
override string getGeneratorName() { result = generatorName }
85+
86+
// If a call matches several `randomNumberGeneratorModel` rows with conflicting
87+
// `secure` values (e.g. a downstream pack reclassifies a generator), the secure
88+
// classification wins: this holds as soon as any matching row has `secure = true`.
89+
// Rows should therefore agree on the security of a given generator.
90+
override predicate isCryptographicallySecure() { secure = true }
91+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/cpp-all
4+
extensible: randomNumberGeneratorModel
5+
# namespace, type, name, output, secure
6+
data:
7+
# C standard library / POSIX / BSD generators returning the value (insecure).
8+
- ["", "", "rand", "", false]
9+
- ["", "", "random", "", false]
10+
- ["", "", "drand48", "", false]
11+
- ["", "", "erand48", "", false]
12+
- ["", "", "lrand48", "", false]
13+
- ["", "", "nrand48", "", false]
14+
- ["", "", "mrand48", "", false]
15+
- ["", "", "jrand48", "", false]
16+
- ["", "", "rand_r", "", false]
17+
# POSIX/BSD generators returning the value (secure).
18+
- ["", "", "arc4random", "", true]
19+
- ["", "", "arc4random_uniform", "", true]
20+
# Generators writing to a buffer argument (secure).
21+
- ["", "", "arc4random_buf", "0", true]
22+
- ["", "", "getrandom", "0", true]
23+
- ["", "", "getentropy", "0", true]
24+
- ["", "", "RtlGenRandom", "0", true]
25+
- ["", "", "BCryptGenRandom", "1", true]
26+
- ["", "", "CryptGenRandom", "2", true]
27+
# C++ <random> engines (insecure) and std::random_device (secure).
28+
- ["std", "mersenne_twister_engine", "operator()", "", false]
29+
- ["std", "linear_congruential_engine", "operator()", "", false]
30+
- ["std", "subtract_with_carry_engine", "operator()", "", false]
31+
- ["std", "discard_block_engine", "operator()", "", false]
32+
- ["std", "shuffle_order_engine", "operator()", "", false]
33+
- ["std", "independent_bits_engine", "operator()", "", false]
34+
- ["std", "random_device", "operator()", "", true]
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#include <openssl/evp.h>
2+
#include <openssl/rand.h>
3+
#include <stdlib.h>
4+
5+
void encrypt(EVP_CIPHER_CTX *ctx, unsigned char *iv) {
6+
unsigned char key[16];
7+
8+
// BAD: the key is derived from a cryptographically weak generator, so an
9+
// attacker may be able to predict it.
10+
for (int i = 0; i < 16; i++) {
11+
key[i] = (unsigned char)rand();
12+
}
13+
EVP_EncryptInit_ex(ctx, EVP_aes_128_cbc(), 0, key, iv);
14+
15+
// GOOD: the key is filled from a cryptographically secure generator.
16+
RAND_bytes(key, 16);
17+
EVP_EncryptInit_ex(ctx, EVP_aes_128_cbc(), 0, key, iv);
18+
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
Using a cryptographically weak pseudo-random number generator to produce a security-sensitive value,
8+
such as an encryption key, an initialization vector, a nonce, or a session token, may allow an attacker
9+
to predict the value.
10+
</p>
11+
12+
<p>
13+
A pseudo-random number generator produces a sequence of numbers that only approximates the properties of
14+
random numbers. The sequence is completely determined by a relatively small seed value. Generators such as
15+
<code>rand</code>, the <code>drand48</code> family, and the C++ <code>&lt;random&gt;</code> engines
16+
(for example <code>std::mt19937</code>) are not designed to resist prediction, so an attacker who observes
17+
some output, or who can reconstruct the seed, may be able to predict future values.
18+
</p>
19+
</overview>
20+
21+
<recommendation>
22+
<p>
23+
Use a cryptographically secure random number generator when the output is used in a security-sensitive
24+
context. Suitable choices include <code>getrandom</code>, <code>getentropy</code>, the
25+
<code>arc4random</code> family, OpenSSL's <code>RAND_bytes</code>, the Windows
26+
<code>BCryptGenRandom</code> function, and C++'s <code>std::random_device</code> (where it is backed by a
27+
secure source).
28+
</p>
29+
</recommendation>
30+
31+
<example>
32+
<p>
33+
The following example seeds an AES key with <code>rand</code>. Because <code>rand</code> is not
34+
cryptographically secure, an attacker may be able to predict the key.
35+
</p>
36+
37+
<sample src="InsecureRandomness.c" />
38+
39+
<p>
40+
Instead, fill the key from a cryptographically secure generator such as <code>RAND_bytes</code>.
41+
</p>
42+
</example>
43+
44+
<references>
45+
<li>Wikipedia:
46+
<a href="https://en.wikipedia.org/wiki/Pseudorandom_number_generator">Pseudorandom number generator</a>.</li>
47+
<li>Common Weakness Enumeration:
48+
<a href="https://cwe.mitre.org/data/definitions/330.html">CWE-330: Use of Insufficiently Random Values</a>.</li>
49+
<li>Common Weakness Enumeration:
50+
<a href="https://cwe.mitre.org/data/definitions/338.html">CWE-338: Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG)</a>.</li>
51+
</references>
52+
</qhelp>
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/**
2+
* @name Insecure randomness
3+
* @description Using a cryptographically insecure pseudo-random number generator to generate a
4+
* security-sensitive value may allow an attacker to predict what value will
5+
* be generated.
6+
* @kind path-problem
7+
* @problem.severity warning
8+
* @security-severity 7.8
9+
* @precision medium
10+
* @id cpp/insecure-randomness
11+
* @tags security
12+
* external/cwe/cwe-330
13+
* external/cwe/cwe-338
14+
*/
15+
16+
import cpp
17+
import experimental.quantum.Language
18+
import InsecureRandomnessFlow::PathGraph
19+
20+
/**
21+
* A taint-tracking configuration for flow from a cryptographically insecure
22+
* random number generator to security-sensitive value such as a key, IV, or nonce.
23+
*/
24+
module InsecureRandomnessConfig implements DataFlow::ConfigSig {
25+
predicate isSource(DataFlow::Node source) {
26+
exists(Crypto::RandomNumberGenerationInstance generator |
27+
not generator.isCryptographicallySecure() and
28+
source = generator.getOutputNode()
29+
)
30+
}
31+
32+
predicate isSink(DataFlow::Node sink) {
33+
sink = any(Crypto::KeyOperationInstance op).getKeyConsumer()
34+
or
35+
sink = any(Crypto::KeyOperationInstance op).getNonceConsumer()
36+
or
37+
sink = any(Crypto::KeyGenerationOperationInstance op).getKeyValueConsumer()
38+
}
39+
40+
predicate isBarrierIn(DataFlow::Node node) { isSource(node) }
41+
42+
predicate isBarrierOut(DataFlow::Node node) { isSink(node) }
43+
44+
predicate observeDiffInformedIncrementalMode() { any() }
45+
}
46+
47+
module InsecureRandomnessFlow = TaintTracking::Global<InsecureRandomnessConfig>;
48+
49+
from InsecureRandomnessFlow::PathNode source, InsecureRandomnessFlow::PathNode sink
50+
where InsecureRandomnessFlow::flowPath(source, sink)
51+
select sink.getNode(), source, sink,
52+
"This security-sensitive value depends on $@, which is not cryptographically secure.",
53+
source.getNode(), "a randomly generated number"
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new query, `cpp/insecure-randomness` ("Insecure randomness"), which flags cryptographically insecure random numbers (for example from `rand` or `std::mt19937`) that are used as security-sensitive values such as encryption keys, IVs, or nonces.

0 commit comments

Comments
 (0)