Skip to content

Commit 17cd22f

Browse files
geoffw0geoffw0
authored
Merge pull request #14972 from geoffw0/cryptoprimitives
C++: Experimental query for implementation of a cryptographic primitive
2 parents 29a1cd1 + 521d98e commit 17cd22f

File tree

6 files changed

+356
-0
lines changed

6 files changed

+356
-0
lines changed

‎cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/**
2+
* @name Implementation of a cryptographic primitive
3+
* @description Writing your own cryptographic primitives is prone to errors and omissions that weaken cryptographic protection.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @security-severity 7.5
7+
* @precision medium
8+
* @id cpp/crypto-primitive
9+
* @tags security
10+
* experimental
11+
* external/cwe/cwe-1240
12+
*/
13+
14+
import cpp
15+
16+
/**
17+
* Gets a word that might be in the name of an encryption function.
18+
*/
19+
string encryptionWord() {
20+
exists(string word |
21+
// `(?<!P)` is negative lookbehind, i.e. the match is not preceded by `P`.
22+
// `(?!P)` is negative lookahead, i.e. the match is not followed by `P`.
23+
word =
24+
[
25+
"Crypt", "Cipher", "Aes", "Rijndael",
26+
//"(?<!Wi|Co|No)Des(?!truct)",
27+
"(?<!C)Rc[0-9]", "(?<!Cha|Unive)Rsa", "Blowfish", "Twofish", "Idea", "Kyber", "(?<!V)Aria",
28+
//"Asn[0-9]",
29+
"Camellia",
30+
//"(?<!Bit|Type)Cast",
31+
"Chacha", "ChaCha", "Poly[0-9]", "Ripemd", "Whirlpool", "Sbox", "SBox", "Cblock", "CBlock",
32+
"Sub.?Bytes?", "Mix.?Columns?", "ECDH", "ECDSA", "EdDSA", "ECMQV", "ECQV", "Curve[0-9][0-9]"
33+
] and
34+
(
35+
result = word or
36+
result = word.toLowerCase() + "(?![a-z])" or // avoid matching middles of words
37+
result = word.toUpperCase() + "(?![A-Z])" // avoid matching middles of words
38+
)
39+
)
40+
}
41+
42+
/**
43+
* Holds if `f` is a function whose name suggests it may be doing encryption
44+
* (but may or may not actually implement an encryption primitive itself).
45+
*/
46+
predicate likelyEncryptionFunction(Function f) {
47+
exists(string fName | fName = f.getName() |
48+
fName.regexpMatch(".*(" + concat(encryptionWord(), "|") + ").*")
49+
)
50+
}
51+
52+
/**
53+
* Holds if `t` is a type that is common in encryption-like computations. That
54+
* is, an integral type or array of integral type elements.
55+
*/
56+
predicate computeHeuristicType(Type t) {
57+
t instanceof IntegralType or
58+
computeHeuristicType(t.(ArrayType).getBaseType().getUnspecifiedType())
59+
}
60+
61+
/**
62+
* Holds if `e` is an operation that is common in encryption-like computations.
63+
* Looking for clusters of these tends to find things like encrpytion,
64+
* compression, random number generation, graphics processing and other compute
65+
* heavy algorithms.
66+
*/
67+
predicate computeHeuristic(Expr e) {
68+
(
69+
e instanceof BitwiseXorExpr or
70+
e instanceof AssignXorExpr or
71+
e instanceof LShiftExpr or
72+
e instanceof AssignLShiftExpr or
73+
e instanceof RShiftExpr or
74+
e instanceof AssignRShiftExpr or
75+
e instanceof ArrayExpr
76+
) and
77+
computeHeuristicType(e.getUnspecifiedType())
78+
}
79+
80+
/**
81+
* Gets the name of an established cryptography library or likely third party directory.
82+
*/
83+
string encryptionLibraryName() {
84+
result =
85+
[
86+
"libssh", "openssl", "boringssl", "mbed", "libsodium", "libsrtp", "third.?party", "library",
87+
"deps"
88+
]
89+
}
90+
91+
/**
92+
* Holds if `f` is a file that is likely to be inside an established
93+
* cryptography library.
94+
*/
95+
predicate isLibrary(File f) {
96+
f.getAbsolutePath().regexpMatch("(?i).*(" + concat(encryptionLibraryName(), "|") + ").*")
97+
or
98+
// assume that any result that would be found outside the source location is in a crypto library
99+
not exists(f.getFile().getRelativePath())
100+
}
101+
102+
from Function f, int amount
103+
where
104+
likelyEncryptionFunction(f) and
105+
amount = strictcount(Expr e | computeHeuristic(e) and e.getEnclosingFunction() = f) and
106+
amount >= 8 and
107+
not isLibrary(f.getFile())
108+
select f,
109+
"This function, \"" + f.getName() +
110+
"\", may be a custom implementation of a cryptographic primitive."

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/CustomCryptographicPrimitive.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| tests_crypto.cpp:11:6:11:18 | encryptString | This function, "encryptString", may be a custom implementation of a cryptographic primitive. |
2+
| tests_crypto.cpp:30:6:30:14 | MyEncrypt | This function, "MyEncrypt", may be a custom implementation of a cryptographic primitive. |
3+
| tests_crypto.cpp:51:6:51:16 | mix_columns | This function, "mix_columns", may be a custom implementation of a cryptographic primitive. |
4+
| tests_crypto.cpp:83:6:83:18 | init_aes_sbox | This function, "init_aes_sbox", may be a custom implementation of a cryptographic primitive. |

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/CustomCryptographicPrimitive.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/library/tests_library.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// Cryptography 'library' snippets. Nothing in this file should be flagged by the query, because
2+
// it's in a library.
3+
4+
void do_aes_encrypt(unsigned int *v) {
5+
COMPUTE(v)
6+
}

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/tests_crypto.cpp

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// Cryptography snippets. All (non-stub) functions in this file should be flagged by the query.
2+
3+
typedef unsigned char uint8_t;
4+
5+
int strlen(const char *string);
6+
7+
// ---
8+
9+
// the following function is homebrew crypto written for this test. This is a bad algorithm
10+
// on multiple levels and should never be used in cryptography.
11+
void encryptString(char *string, unsigned int key) {
12+
char *ptr = string;
13+
int len = strlen(string);
14+
15+
while (len >= 4) {
16+
// encrypt block by XOR-ing with the key
17+
ptr[0] = ptr[0] ^ (key >> 0);
18+
ptr[1] = ptr[1] ^ (key >> 8);
19+
ptr[2] = ptr[2] ^ (key >> 16);
20+
ptr[3] = ptr[3] ^ (key >> 24);
21+
22+
// move on
23+
ptr += 4;
24+
len -= 4;
25+
}
26+
}
27+
28+
// the following function is homebrew crypto written for this test. This is a bad algorithm
29+
// on multiple levels and should never be used in cryptography.
30+
void MyEncrypt(const unsigned int *dataIn, unsigned int *dataOut, unsigned int dataSize, unsigned int key[2]) {
31+
unsigned int state[2];
32+
unsigned int t;
33+
34+
state[0] = key[0];
35+
state[1] = key[1];
36+
37+
for (unsigned int i = 0; i < dataSize; i++) {
38+
// mix state
39+
t = state[0];
40+
state[0] = (state[0] << 1) | (state[1] >> 31);
41+
state[1] = (state[1] << 1) | (t >> 31);
42+
43+
// encrypt data
44+
dataOut[i] = dataIn[i] ^ state[0];
45+
}
46+
}
47+
48+
// the following function resembles an implementation of the AES "mix columns"
49+
// step. It is not accurate, efficient or safe and should never be used in
50+
// cryptography.
51+
void mix_columns(const uint8_t inputs[4], uint8_t outputs[4]) {
52+
// The "mix columns" step takes four bytes as inputs. Each byte represents a
53+
// polynomial with 8 one-bit coefficients, e.g. input bits 00001101
54+
// represent the polynomial x^3 + x^2 + 1. Arithmetic is reduced modulo
55+
// x^8 + x^4 + x^3 + x + 1 (= 0x11b).
56+
//
57+
// The "mix columns" step multiplies each input by 2 (in the field described
58+
// above) to produce four more values. The output is then four values
59+
// produced by XOR-ing specific combinations of five of these eight values.
60+
// The exact values selected here do not match the actual AES algorithm.
61+
//
62+
// We avoid control flow decisions that depend on the inputs.
63+
uint8_t vs[4];
64+
65+
vs[0] = inputs[0] << 1; // multiply by two
66+
vs[0] ^= (inputs[0] >> 7) * 0x1b; // reduce modulo 0x11b; the top bit was removed in the shift.
67+
vs[1] = inputs[1] << 1;
68+
vs[1] ^= (inputs[1] >> 7) * 0x1b;
69+
vs[2] = inputs[2] << 1;
70+
vs[2] ^= (inputs[2] >> 7) * 0x1b;
71+
vs[3] = inputs[3] << 1;
72+
vs[3] ^= (inputs[3] >> 7) * 0x1b;
73+
74+
outputs[0] = inputs[0] ^ inputs[1] ^ inputs[2] ^ vs[0] ^ vs[1];
75+
outputs[1] = inputs[1] ^ inputs[2] ^ inputs[3] ^ vs[1] ^ vs[2];
76+
outputs[2] = inputs[2] ^ inputs[3] ^ inputs[0] ^ vs[2] ^ vs[3];
77+
outputs[3] = inputs[3] ^ inputs[0] ^ inputs[1] ^ vs[3] ^ vs[0];
78+
}
79+
80+
// the following function resembles initialization of an S-box as may be done
81+
// in an implementation of DES, AES and other encryption algorithms. It is not
82+
// accurate, efficient or safe and should never be used in cryptography.
83+
void init_aes_sbox(unsigned char data[256]) {
84+
// initialize `data` in a loop using lots of ^, ^= and << operations and
85+
// a few fixed constants.
86+
unsigned int state = 0x12345678;
87+
88+
for (int i = 0; i < 256; i++)
89+
{
90+
state ^= (i ^ 0x86) << 24;
91+
state ^= (i ^ 0xb9) << 16;
92+
state ^= (i ^ 0x11) << 8;
93+
state ^= (i ^ 0x23) << 0;
94+
state = (state << 1) ^ (state >> 31);
95+
data[i] = state & 0xff;
96+
}
97+
}

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/tests_not_crypto.cpp

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
// Non-cryptography snippets. Nothing in this file should be flagged by the query.
2+
3+
typedef unsigned char uint8_t;
4+
typedef unsigned int uint32_t;
5+
typedef unsigned long size_t;
6+
7+
// a very cut down stub for `std::cout`
8+
namespace std
9+
{
10+
template<class charT> struct char_traits;
11+
12+
template <class charT, class traits = char_traits<charT> >
13+
class basic_ostream {
14+
public:
15+
typedef charT char_type;
16+
};
17+
template<class charT, class traits> basic_ostream<charT,traits>& operator<<(basic_ostream<charT,traits>&, const charT*);
18+
19+
typedef basic_ostream<char> ostream;
20+
21+
extern ostream cout;
22+
}
23+
24+
// this macro expands to some compute operations that look a bit like cryptography
25+
#define COMPUTE(v) \
26+
v[0] ^= v[1] ^ v[2] ^ v[3] ^ v[4]; \
27+
v[1] ^= v[2] ^ v[3] ^ v[4] ^ v[5]; \
28+
v[2] ^= v[3] ^ v[4] ^ v[5] ^ v[6]; \
29+
v[3] ^= v[4] ^ v[5] ^ v[6] ^ v[7];
30+
31+
// ---
32+
33+
#include "library/tests_library.h"
34+
35+
bool isEnabledAes() {
36+
// This function has "Aes" in it's name, but does not contain enough compute to
37+
// be an encryption implementation.
38+
return false;
39+
}
40+
41+
uint32_t lookup[256];
42+
43+
uint8_t computeCRC32(const uint8_t *data, size_t dataLen) {
44+
// This function has "RC3" in its name, but is not an implementation of the (broken) RC3 encryption algorithm.
45+
uint32_t result = 0xFFFFFFFF;
46+
47+
for (size_t i = 0; i < dataLen; i++) {
48+
result = (result >> 8) + lookup[(result ^ data[i]) & 0xFF];
49+
result = (result >> 8) + lookup[(result ^ data[i]) & 0xFF]; // artificial extra compute
50+
result = (result >> 8) + lookup[(result ^ data[i]) & 0xFF]; // artificial extra compute
51+
result = (result >> 8) + lookup[(result ^ data[i]) & 0xFF]; // artificial extra compute
52+
}
53+
54+
return result ^ 0xFFFFFFFF;
55+
}
56+
57+
void convert_image_universal(uint32_t *img, int width, int height) {
58+
// This function has "rsa" in its name, but is nothing to do with the RSA encryption algorithm.
59+
uint32_t *pixel_ptr = img;
60+
uint32_t num_pixels = width * height;
61+
62+
// convert pixels RGBA -> ARGB (with probably unhelpful loop unrolling)
63+
while (num_pixels >= 4) {
64+
pixel_ptr[0] = (pixel_ptr[0] >> 8) ^ (pixel_ptr[0] << 24);
65+
pixel_ptr[1] = (pixel_ptr[1] >> 8) ^ (pixel_ptr[1] << 24);
66+
pixel_ptr[2] = (pixel_ptr[2] >> 8) ^ (pixel_ptr[2] << 24);
67+
pixel_ptr[3] = (pixel_ptr[3] >> 8) ^ (pixel_ptr[3] << 24);
68+
num_pixels -= 4;
69+
}
70+
if (num_pixels >= 2) {
71+
pixel_ptr[0] = (pixel_ptr[0] >> 8) ^ (pixel_ptr[0] << 24);
72+
pixel_ptr[1] = (pixel_ptr[1] >> 8) ^ (pixel_ptr[1] << 24);
73+
num_pixels -= 2;
74+
}
75+
if (num_pixels >= 1) {
76+
pixel_ptr[2] = (pixel_ptr[2] >> 8) ^ (pixel_ptr[2] << 24);
77+
}
78+
}
79+
80+
const char* yes_no_setting() { return "no"; }
81+
82+
void output_encrypt_decrypt_algorithms() {
83+
// This function has "encrypt" and "decrypt" in its name, but no encryption is done.
84+
// This function uses `<<` heavily, but not as an integer shift left.
85+
const char *indent = " ";
86+
87+
std::cout << "Supported algorithms:\n";
88+
std::cout << indent << "DES (" << yes_no_setting() << ")\n";
89+
std::cout << indent << "3DES (" << yes_no_setting() << ")\n";
90+
std::cout << indent << "AES (" << yes_no_setting() << ")\n";
91+
std::cout << indent << "RSA (" << yes_no_setting() << ")\n";
92+
std::cout << indent << "Blowfish (" << yes_no_setting() << ")\n";
93+
std::cout << indent << "Twofish (" << yes_no_setting() << ")\n";
94+
std::cout << indent << "Chacha (" << yes_no_setting() << ")\n";
95+
}
96+
97+
void wideStringCharsAt(int *v) {
98+
// This function has "des" and "rsa" in the name.
99+
COMPUTE(v)
100+
}
101+
102+
void bitcastVariable(int *v) {
103+
// This function has "aria" and "cast" in the name.
104+
COMPUTE(v)
105+
}
106+
107+
void dividesVariance(int *v) {
108+
// This function has "des" and "aria" in the name.
109+
COMPUTE(v)
110+
}
111+
112+
void broadcastNodes(int *v) {
113+
// This function has "cast" and "des" in the name.
114+
COMPUTE(v)
115+
}
116+
117+
#define ROTATE(val, amount) ( (val << amount) | (val >> (32 - amount)) )
118+
119+
static inline void hashMix(const int *data, int &state) {
120+
// This function looks like part of a hashing function. It's not necessarily intended to
121+
// be a cryptographic hash, so should not be flagged.
122+
state ^= data[0];
123+
ROTATE(state, 1);
124+
state ^= data[1];
125+
ROTATE(state, 7);
126+
state ^= data[2];
127+
ROTATE(state, 11);
128+
state ^= data[3];
129+
ROTATE(state, 3);
130+
state ^= data[4];
131+
ROTATE(state, 13);
132+
state ^= data[5];
133+
ROTATE(state, 5);
134+
state ^= data[6];
135+
ROTATE(state, 2);
136+
state ^= data[7];
137+
ROTATE(state, 17);
138+
}

0 commit comments

Comments
 (0)