Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* 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.
1 change: 1 addition & 0 deletions cpp/ql/lib/experimental/quantum/Language.qll
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,4 @@ private class ConstantDataSource extends Crypto::GenericConstantSourceInstance i
}

import OpenSSL.OpenSSL
import Standard.Random
8 changes: 7 additions & 1 deletion cpp/ql/lib/experimental/quantum/OpenSSL/Random.qll
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,18 @@ private import semmle.code.cpp.dataflow.new.DataFlow
class OpenSslRandomNumberGeneratorInstance extends Crypto::RandomNumberGenerationInstance instanceof Call
{
OpenSslRandomNumberGeneratorInstance() {
this.(Call).getTarget().getName() in ["RAND_bytes", "RAND_pseudo_bytes"]
this.(Call).getTarget().getName() in ["RAND_bytes", "RAND_priv_bytes", "RAND_pseudo_bytes"]
}

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

override string getGeneratorName() { result = this.(Call).getTarget().getName() }

override predicate isCryptographicallySecure() {
// `RAND_pseudo_bytes` is deprecated and does not guarantee cryptographically
// secure output, so it is deliberately excluded here.
this.(Call).getTarget().getName() in ["RAND_bytes", "RAND_priv_bytes"]
}
}
91 changes: 91 additions & 0 deletions cpp/ql/lib/experimental/quantum/Standard/Random.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/**
* Models random number generation from the C standard library, POSIX/BSD, the
* Windows CryptoAPI/CNG, and the C++ `<random>` engines, as instances of the
* shared quantum `Crypto::RandomNumberGenerationInstance` concept.
*
* The set of modelled generators is defined as data through the
* `randomNumberGeneratorModel` extensible predicate, so that downstream packs can
* register additional generators without editing this library. Each row records
* whether the generator is cryptographically secure; insecure generators (e.g.
* `rand`, `std::mt19937`) leave `isCryptographicallySecure()` at its default of
* holding for no generator.
*
* Only functions that *produce* random output are modelled here. Seeding
* functions such as `srand`, `srandom`, `srand48`, and `seed48` produce no output
* artifact and are therefore out of scope for this concept.
*/

import cpp
private import experimental.quantum.Language

/**
* Holds if a call to the function `name` is a random number generator.
*
* `namespace` and `type` identify the function: when `type` is empty, `name` is a
* global or `std` free function (e.g. `rand`); otherwise `name` is a member
* function of the class (template) whose unqualified name is `type` (e.g.
* `operator()` of `std::mersenne_twister_engine`).
*
* `output` is the index of the argument into which the random bytes are written,
* or the empty string if the random value is the return value.
*
* `secure` holds if the generator is cryptographically secure.
*/
extensible predicate randomNumberGeneratorModel(
string namespace, string type, string name, string output, boolean secure
);

/**
* Holds if `c` is a call to a modelled random number generator named
* `generatorName`, writing its output as described by `output` (see
* `randomNumberGeneratorModel`), where `secure` holds if it is cryptographically
* secure.
*/
private predicate randomNumberGeneratorCall(
Call c, string generatorName, string output, boolean secure
) {
exists(string namespace, string type, string name, Function f |
randomNumberGeneratorModel(namespace, type, name, output, secure) and
f = c.getTarget()
|
// A global or `std` free function, e.g. `rand` or `std::rand`.
type = "" and
f.hasGlobalOrStdName(name) and
generatorName = name
or
// A member function of a class (template), e.g. `std::mt19937::operator()`.
type != "" and
f.getName() = name and
f.getDeclaringType().getSimpleName() = type and
(if namespace = "" then generatorName = type else generatorName = namespace + "::" + type)
Comment on lines +51 to +60
)
}

/**
* A call to a random number generator modelled through the `randomNumberGeneratorModel`
* extensible predicate.
*/
class ModeledRandomNumberGeneratorInstance extends Crypto::RandomNumberGenerationInstance instanceof Call
{
string generatorName;
string output;
boolean secure;

ModeledRandomNumberGeneratorInstance() {
randomNumberGeneratorCall(this, generatorName, output, secure)
}

override Crypto::DataFlowNode getOutputNode() {
output = "" and result.asExpr() = this
or
output != "" and result.asDefiningArgument() = super.getArgument(output.toInt())
}

override string getGeneratorName() { result = generatorName }

// If a call matches several `randomNumberGeneratorModel` rows with conflicting
// `secure` values (e.g. a downstream pack reclassifies a generator), the secure
// classification wins: this holds as soon as any matching row has `secure = true`.
// Rows should therefore agree on the security of a given generator.
override predicate isCryptographicallySecure() { secure = true }
}
34 changes: 34 additions & 0 deletions cpp/ql/lib/ext/experimental.quantum.Random.model.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
extensions:
- addsTo:
pack: codeql/cpp-all
extensible: randomNumberGeneratorModel
# namespace, type, name, output, secure
data:
# C standard library / POSIX / BSD generators returning the value (insecure).
- ["", "", "rand", "", false]
- ["", "", "random", "", false]
- ["", "", "drand48", "", false]
- ["", "", "erand48", "", false]
- ["", "", "lrand48", "", false]
- ["", "", "nrand48", "", false]
- ["", "", "mrand48", "", false]
- ["", "", "jrand48", "", false]
- ["", "", "rand_r", "", false]
# POSIX/BSD generators returning the value (secure).
- ["", "", "arc4random", "", true]
- ["", "", "arc4random_uniform", "", true]
# Generators writing to a buffer argument (secure).
- ["", "", "arc4random_buf", "0", true]
- ["", "", "getrandom", "0", true]
- ["", "", "getentropy", "0", true]
- ["", "", "RtlGenRandom", "0", true]
- ["", "", "BCryptGenRandom", "1", true]
- ["", "", "CryptGenRandom", "2", true]
# C++ <random> engines (insecure) and std::random_device (secure).
- ["std", "mersenne_twister_engine", "operator()", "", false]
- ["std", "linear_congruential_engine", "operator()", "", false]
- ["std", "subtract_with_carry_engine", "operator()", "", false]
- ["std", "discard_block_engine", "operator()", "", false]
- ["std", "shuffle_order_engine", "operator()", "", false]
- ["std", "independent_bits_engine", "operator()", "", false]
- ["std", "random_device", "operator()", "", true]
18 changes: 18 additions & 0 deletions cpp/ql/src/Security/CWE/CWE-330/InsecureRandomness.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#include <openssl/evp.h>
#include <openssl/rand.h>
#include <stdlib.h>

void encrypt(EVP_CIPHER_CTX *ctx, unsigned char *iv) {
unsigned char key[16];

// BAD: the key is derived from a cryptographically weak generator, so an
// attacker may be able to predict it.
for (int i = 0; i < 16; i++) {
key[i] = (unsigned char)rand();
}
EVP_EncryptInit_ex(ctx, EVP_aes_128_cbc(), 0, key, iv);

// GOOD: the key is filled from a cryptographically secure generator.
RAND_bytes(key, 16);
EVP_EncryptInit_ex(ctx, EVP_aes_128_cbc(), 0, key, iv);
}
52 changes: 52 additions & 0 deletions cpp/ql/src/Security/CWE/CWE-330/InsecureRandomness.qhelp
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
Using a cryptographically weak pseudo-random number generator to produce a security-sensitive value,
such as an encryption key, an initialization vector, a nonce, or a session token, may allow an attacker
to predict the value.
</p>

<p>
A pseudo-random number generator produces a sequence of numbers that only approximates the properties of
random numbers. The sequence is completely determined by a relatively small seed value. Generators such as
<code>rand</code>, the <code>drand48</code> family, and the C++ <code>&lt;random&gt;</code> engines
(for example <code>std::mt19937</code>) are not designed to resist prediction, so an attacker who observes
some output, or who can reconstruct the seed, may be able to predict future values.
</p>
</overview>

<recommendation>
<p>
Use a cryptographically secure random number generator when the output is used in a security-sensitive
context. Suitable choices include <code>getrandom</code>, <code>getentropy</code>, the
<code>arc4random</code> family, OpenSSL's <code>RAND_bytes</code>, the Windows
<code>BCryptGenRandom</code> function, and C++'s <code>std::random_device</code> (where it is backed by a
secure source).
</p>
</recommendation>

<example>
<p>
The following example seeds an AES key with <code>rand</code>. Because <code>rand</code> is not
cryptographically secure, an attacker may be able to predict the key.
</p>

<sample src="InsecureRandomness.c" />

<p>
Instead, fill the key from a cryptographically secure generator such as <code>RAND_bytes</code>.
</p>
</example>

<references>
<li>Wikipedia:
<a href="https://en.wikipedia.org/wiki/Pseudorandom_number_generator">Pseudorandom number generator</a>.</li>
<li>Common Weakness Enumeration:
<a href="https://cwe.mitre.org/data/definitions/330.html">CWE-330: Use of Insufficiently Random Values</a>.</li>
<li>Common Weakness Enumeration:
<a href="https://cwe.mitre.org/data/definitions/338.html">CWE-338: Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG)</a>.</li>
</references>
</qhelp>
53 changes: 53 additions & 0 deletions cpp/ql/src/Security/CWE/CWE-330/InsecureRandomness.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* @name Insecure randomness
* @description Using a cryptographically insecure pseudo-random number generator to generate a
* security-sensitive value may allow an attacker to predict what value will
* be generated.
* @kind path-problem
* @problem.severity warning
* @security-severity 7.8
* @precision medium
* @id cpp/insecure-randomness
* @tags security
* external/cwe/cwe-330
* external/cwe/cwe-338
*/

import cpp
import experimental.quantum.Language
import InsecureRandomnessFlow::PathGraph

/**
* A taint-tracking configuration for flow from a cryptographically insecure
* random number generator to security-sensitive value such as a key, IV, or nonce.
*/
module InsecureRandomnessConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
exists(Crypto::RandomNumberGenerationInstance generator |
not generator.isCryptographicallySecure() and
source = generator.getOutputNode()
)
}

predicate isSink(DataFlow::Node sink) {
sink = any(Crypto::KeyOperationInstance op).getKeyConsumer()
or
sink = any(Crypto::KeyOperationInstance op).getNonceConsumer()
or
sink = any(Crypto::KeyGenerationOperationInstance op).getKeyValueConsumer()
}

predicate isBarrierIn(DataFlow::Node node) { isSource(node) }

predicate isBarrierOut(DataFlow::Node node) { isSink(node) }

predicate observeDiffInformedIncrementalMode() { any() }
}

module InsecureRandomnessFlow = TaintTracking::Global<InsecureRandomnessConfig>;

from InsecureRandomnessFlow::PathNode source, InsecureRandomnessFlow::PathNode sink
where InsecureRandomnessFlow::flowPath(source, sink)
select sink.getNode(), source, sink,
"This security-sensitive value depends on $@, which is not cryptographically secure.",
source.getNode(), "a randomly generated number"
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: newQuery
---
* 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.
Loading