Skip to content

Commit ed69543

Browse files
committed
Added documentation for crypto queries
1 parent 41c6bc1 commit ed69543

11 files changed

+171
-2
lines changed
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,23 @@
11
# Custom allocator leak
2+
3+
This query identifies potential memory leaks from custom allocators like
4+
`BN_new`.
5+
6+
The following example would be identified by the query as a potential memory
7+
leak.
8+
9+
```cpp
10+
int compute(BIGNUM* a) {
11+
BIGNUM *b = BN_new();
12+
13+
// Perform computation on `a` and `b`.
14+
15+
if (condition(a)) {
16+
BN_free(b);
17+
return a;
18+
}
19+
20+
// The BIGNUM `b` may leak here.
21+
return a;
22+
}
23+
```
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,23 @@
11
# Custom allocator use-after-free
2+
3+
This query identifies use-after-frees with custom allocators like `BN_new`.
4+
5+
The following code snippet would be identified as an issue by the query.
6+
7+
```cpp
8+
BIGNUM* compute(BIGNUM* a) {
9+
BIGNUM *b = BN_new();
10+
11+
if (condition(a)) {
12+
// The BIGNUM `b` may be freed here.
13+
BN_free(b);
14+
}
15+
// Potential use-after-free of `b` here.
16+
if (condition(b)) {
17+
BN_free(a);
18+
a = BN_new();
19+
}
20+
21+
return b;
22+
}
23+
```
Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,18 @@
1-
# RAND_bytes and BN_rand error handling
1+
# Checking if returned error codes are acted on
2+
3+
Many of the functions comprising the OpenSSL API return an integer error code
4+
where 1 typically signals success, and 0 signals failure. To ensure that the
5+
implementation is secure, these error codes must be checked and acted upon. This
6+
query attempts to identify locations where a returned error code is not checked
7+
by the codebase. In this context, checked means that the return value flows
8+
to the condition of a control-flow statement like an if-statement, a while-
9+
statement, or a switch-statement.
10+
11+
As an example, the following example function fails to check the return value
12+
from `RAND_bytes`, and would be flagged as an issue by the query.
13+
14+
```cpp
15+
void generateEncryptionKey(unsigned char *key, size_t keySize) {
16+
RAND_bytes(key, keySize);
17+
}
18+
```
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,19 @@
11
# Invalid key size
2+
3+
The `EVP_EncryptInit` and `EVP_EncryptInit_ex` functions do not take the size
4+
of the key as input, and the implementation will simply read the expected number
5+
of bytes from the buffer. This query will check if the size of the key buffer
6+
passed as an argument to one of these functions is equal to the key size of the
7+
corresponding cipher.
8+
9+
The following code snippet is aan example of an issue that would be identified
10+
by the query.
11+
12+
```cpp
13+
unsigned char key[16]; // This should be 32 for a 256 bit key
14+
RAND_bytes(key, sizeof(key));
15+
16+
unsigned char *iv = (unsigned char *)"0123456789ABCDEF"; // A fixed 16 byte IV
17+
18+
EVP_EncryptInit_ex(EVP_CIPHER_CTX_new(), EVP_aes_256_cbc(), NULL, key, iv);
19+
```
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,15 @@
11
# Missing engine initialization
2+
3+
This query identifies loaded OpenSSL engines which are not passed to both
4+
`ENGINE_init` and `ENGINE_set_default`. `ENGINE_init` should always be called
5+
when a new engine is loaded. It is generally good practise to also call
6+
`ENGINE_set_default` to ensure that the primitives defined by the engine are
7+
used by default.
8+
9+
The following code snippet would be flagged as an issue by the query.
10+
11+
```cpp
12+
ENGINE* load_rdrand() {
13+
return ENGINE_by_id("rdrand");
14+
}
15+
```
Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,21 @@
1-
# Non-cleared bignum detection
1+
# Missing zeroization of random BIGNUMs
2+
3+
Randomly generated BIGNUMs often represent sensitive data (e.g. like ECDSA
4+
nonces). These should be cleared as they go out of scope to ensure that
5+
sensitive data does not remain in memory longer than required.
6+
7+
This query identifies OpenSSL BIGNUMs which are inititialized using `BN_rand`
8+
but not which are not zeroized using `BN_clear` before they go out of scope. The
9+
following example function would be flagged as an issue by the query.
10+
11+
```cpp
12+
void compute() {
13+
BIGNUM *n = BN_new();
14+
BN_rand(n, 128, BN_RAND_TOP_ANY, BN_RAND_BOTTOM_ANY);
15+
16+
// Perform sensitive computation using `n`.
17+
18+
BN_free(n);
19+
}
20+
```
21+
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,17 @@
11
# Random buffer too small
2+
3+
This query finds buffer overflows in calls to CSPRNGs like `RAND_bytes`,
4+
`RAND_bytes_ex`, and `RAND_priv_bytes`. It is currently restricted to statically
5+
allocated buffers to allow us to easily determine the input buffer size, but
6+
could easily be extended to dynamically allocated buffers as well.
7+
8+
The following example code would be flagged as vulnerable by the query.
9+
10+
```cpp
11+
#define KEY_SIZE 16
12+
13+
// ...
14+
15+
unsigned char key[KEY_SIZE];
16+
int res = RAND_bytes(key, 32)
17+
```
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,5 @@
11
# Static key flow
2+
3+
This query flags locations where a static buffer is used as an argument to a
4+
function requiring strong, cryptographic level randomness (e.g. an encryption
5+
key).
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,5 @@
11
# Static password flow
2+
3+
This query flags locations where a static string is used as an argument to a
4+
function requiring a strong password (e.g. the password argument for a password-
5+
based key derivation function).
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,26 @@
11
# Legacy cryptographic algorithm
2+
3+
This query finds uses of weak or depracated cryptographic algorithms like `MD5`,
4+
`SHA1`, and `DES`. The query will flag calls to functions containing any of the
5+
following names:
6+
7+
- `MD2`
8+
- `MD4`
9+
- `MD5`
10+
- `RIPEMD`
11+
- `SHA1`
12+
- `Streebog`
13+
- `Whirlpool`
14+
15+
- `PBKDF1`
16+
17+
- `Arcfour`
18+
- `Blowfish`
19+
- `CAST`
20+
- `DES`
21+
- `IDEA`
22+
- `Kasumi`
23+
- `Magma`
24+
- `RC2`
25+
- `RC4`
26+
- `TDEA`

0 commit comments

Comments
 (0)