diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql b/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql new file mode 100644 index 00000000000..751282e0f40 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql @@ -0,0 +1,110 @@ +/** + * @name Implementation of a cryptographic primitive + * @description Writing your own cryptographic primitives is prone to errors and omissions that weaken cryptographic protection. + * @kind problem + * @problem.severity warning + * @security-severity 7.5 + * @precision medium + * @id cpp/crypto-primitive + * @tags security + * experimental + * external/cwe/cwe-1240 + */ + +import cpp + +/** + * Gets a word that might be in the name of an encryption function. + */ +string encryptionWord() { + exists(string word | + // `(?= 8 and + not isLibrary(f.getFile()) +select f, + "This function, \"" + f.getName() + + "\", may be a custom implementation of a cryptographic primitive." diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/CustomCryptographicPrimitive.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/CustomCryptographicPrimitive.expected new file mode 100644 index 00000000000..7be87dcf54c --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/CustomCryptographicPrimitive.expected @@ -0,0 +1,4 @@ +| tests_crypto.cpp:11:6:11:18 | encryptString | This function, "encryptString", may be a custom implementation of a cryptographic primitive. | +| tests_crypto.cpp:30:6:30:14 | MyEncrypt | This function, "MyEncrypt", may be a custom implementation of a cryptographic primitive. | +| tests_crypto.cpp:51:6:51:16 | mix_columns | This function, "mix_columns", may be a custom implementation of a cryptographic primitive. | +| tests_crypto.cpp:83:6:83:18 | init_aes_sbox | This function, "init_aes_sbox", may be a custom implementation of a cryptographic primitive. | diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/CustomCryptographicPrimitive.qlref b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/CustomCryptographicPrimitive.qlref new file mode 100644 index 00000000000..ddf0380834b --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/CustomCryptographicPrimitive.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/library/tests_library.h b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/library/tests_library.h new file mode 100644 index 00000000000..49eae1c3fa7 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/library/tests_library.h @@ -0,0 +1,6 @@ +// Cryptography 'library' snippets. Nothing in this file should be flagged by the query, because +// it's in a library. + +void do_aes_encrypt(unsigned int *v) { + COMPUTE(v) +} diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/tests_crypto.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/tests_crypto.cpp new file mode 100644 index 00000000000..6aa1bbe06a7 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/tests_crypto.cpp @@ -0,0 +1,97 @@ +// Cryptography snippets. All (non-stub) functions in this file should be flagged by the query. + +typedef unsigned char uint8_t; + +int strlen(const char *string); + +// --- + +// the following function is homebrew crypto written for this test. This is a bad algorithm +// on multiple levels and should never be used in cryptography. +void encryptString(char *string, unsigned int key) { + char *ptr = string; + int len = strlen(string); + + while (len >= 4) { + // encrypt block by XOR-ing with the key + ptr[0] = ptr[0] ^ (key >> 0); + ptr[1] = ptr[1] ^ (key >> 8); + ptr[2] = ptr[2] ^ (key >> 16); + ptr[3] = ptr[3] ^ (key >> 24); + + // move on + ptr += 4; + len -= 4; + } +} + +// the following function is homebrew crypto written for this test. This is a bad algorithm +// on multiple levels and should never be used in cryptography. +void MyEncrypt(const unsigned int *dataIn, unsigned int *dataOut, unsigned int dataSize, unsigned int key[2]) { + unsigned int state[2]; + unsigned int t; + + state[0] = key[0]; + state[1] = key[1]; + + for (unsigned int i = 0; i < dataSize; i++) { + // mix state + t = state[0]; + state[0] = (state[0] << 1) | (state[1] >> 31); + state[1] = (state[1] << 1) | (t >> 31); + + // encrypt data + dataOut[i] = dataIn[i] ^ state[0]; + } +} + +// the following function resembles an implementation of the AES "mix columns" +// step. It is not accurate, efficient or safe and should never be used in +// cryptography. +void mix_columns(const uint8_t inputs[4], uint8_t outputs[4]) { + // The "mix columns" step takes four bytes as inputs. Each byte represents a + // polynomial with 8 one-bit coefficients, e.g. input bits 00001101 + // represent the polynomial x^3 + x^2 + 1. Arithmetic is reduced modulo + // x^8 + x^4 + x^3 + x + 1 (= 0x11b). + // + // The "mix columns" step multiplies each input by 2 (in the field described + // above) to produce four more values. The output is then four values + // produced by XOR-ing specific combinations of five of these eight values. + // The exact values selected here do not match the actual AES algorithm. + // + // We avoid control flow decisions that depend on the inputs. + uint8_t vs[4]; + + vs[0] = inputs[0] << 1; // multiply by two + vs[0] ^= (inputs[0] >> 7) * 0x1b; // reduce modulo 0x11b; the top bit was removed in the shift. + vs[1] = inputs[1] << 1; + vs[1] ^= (inputs[1] >> 7) * 0x1b; + vs[2] = inputs[2] << 1; + vs[2] ^= (inputs[2] >> 7) * 0x1b; + vs[3] = inputs[3] << 1; + vs[3] ^= (inputs[3] >> 7) * 0x1b; + + outputs[0] = inputs[0] ^ inputs[1] ^ inputs[2] ^ vs[0] ^ vs[1]; + outputs[1] = inputs[1] ^ inputs[2] ^ inputs[3] ^ vs[1] ^ vs[2]; + outputs[2] = inputs[2] ^ inputs[3] ^ inputs[0] ^ vs[2] ^ vs[3]; + outputs[3] = inputs[3] ^ inputs[0] ^ inputs[1] ^ vs[3] ^ vs[0]; +} + +// the following function resembles initialization of an S-box as may be done +// in an implementation of DES, AES and other encryption algorithms. It is not +// accurate, efficient or safe and should never be used in cryptography. +void init_aes_sbox(unsigned char data[256]) { + // initialize `data` in a loop using lots of ^, ^= and << operations and + // a few fixed constants. + unsigned int state = 0x12345678; + + for (int i = 0; i < 256; i++) + { + state ^= (i ^ 0x86) << 24; + state ^= (i ^ 0xb9) << 16; + state ^= (i ^ 0x11) << 8; + state ^= (i ^ 0x23) << 0; + state = (state << 1) ^ (state >> 31); + data[i] = state & 0xff; + } +} diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/tests_not_crypto.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/tests_not_crypto.cpp new file mode 100644 index 00000000000..c84d4dbba77 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/tests_not_crypto.cpp @@ -0,0 +1,138 @@ +// Non-cryptography snippets. Nothing in this file should be flagged by the query. + +typedef unsigned char uint8_t; +typedef unsigned int uint32_t; +typedef unsigned long size_t; + +// a very cut down stub for `std::cout` +namespace std +{ + template struct char_traits; + + template > + class basic_ostream { + public: + typedef charT char_type; + }; + template basic_ostream& operator<<(basic_ostream&, const charT*); + + typedef basic_ostream ostream; + + extern ostream cout; +} + +// this macro expands to some compute operations that look a bit like cryptography +#define COMPUTE(v) \ + v[0] ^= v[1] ^ v[2] ^ v[3] ^ v[4]; \ + v[1] ^= v[2] ^ v[3] ^ v[4] ^ v[5]; \ + v[2] ^= v[3] ^ v[4] ^ v[5] ^ v[6]; \ + v[3] ^= v[4] ^ v[5] ^ v[6] ^ v[7]; + +// --- + +#include "library/tests_library.h" + +bool isEnabledAes() { + // This function has "Aes" in it's name, but does not contain enough compute to + // be an encryption implementation. + return false; +} + +uint32_t lookup[256]; + +uint8_t computeCRC32(const uint8_t *data, size_t dataLen) { + // This function has "RC3" in its name, but is not an implementation of the (broken) RC3 encryption algorithm. + uint32_t result = 0xFFFFFFFF; + + for (size_t i = 0; i < dataLen; i++) { + result = (result >> 8) + lookup[(result ^ data[i]) & 0xFF]; + result = (result >> 8) + lookup[(result ^ data[i]) & 0xFF]; // artificial extra compute + result = (result >> 8) + lookup[(result ^ data[i]) & 0xFF]; // artificial extra compute + result = (result >> 8) + lookup[(result ^ data[i]) & 0xFF]; // artificial extra compute + } + + return result ^ 0xFFFFFFFF; +} + +void convert_image_universal(uint32_t *img, int width, int height) { + // This function has "rsa" in its name, but is nothing to do with the RSA encryption algorithm. + uint32_t *pixel_ptr = img; + uint32_t num_pixels = width * height; + + // convert pixels RGBA -> ARGB (with probably unhelpful loop unrolling) + while (num_pixels >= 4) { + pixel_ptr[0] = (pixel_ptr[0] >> 8) ^ (pixel_ptr[0] << 24); + pixel_ptr[1] = (pixel_ptr[1] >> 8) ^ (pixel_ptr[1] << 24); + pixel_ptr[2] = (pixel_ptr[2] >> 8) ^ (pixel_ptr[2] << 24); + pixel_ptr[3] = (pixel_ptr[3] >> 8) ^ (pixel_ptr[3] << 24); + num_pixels -= 4; + } + if (num_pixels >= 2) { + pixel_ptr[0] = (pixel_ptr[0] >> 8) ^ (pixel_ptr[0] << 24); + pixel_ptr[1] = (pixel_ptr[1] >> 8) ^ (pixel_ptr[1] << 24); + num_pixels -= 2; + } + if (num_pixels >= 1) { + pixel_ptr[2] = (pixel_ptr[2] >> 8) ^ (pixel_ptr[2] << 24); + } +} + +const char* yes_no_setting() { return "no"; } + +void output_encrypt_decrypt_algorithms() { + // This function has "encrypt" and "decrypt" in its name, but no encryption is done. + // This function uses `<<` heavily, but not as an integer shift left. + const char *indent = " "; + + std::cout << "Supported algorithms:\n"; + std::cout << indent << "DES (" << yes_no_setting() << ")\n"; + std::cout << indent << "3DES (" << yes_no_setting() << ")\n"; + std::cout << indent << "AES (" << yes_no_setting() << ")\n"; + std::cout << indent << "RSA (" << yes_no_setting() << ")\n"; + std::cout << indent << "Blowfish (" << yes_no_setting() << ")\n"; + std::cout << indent << "Twofish (" << yes_no_setting() << ")\n"; + std::cout << indent << "Chacha (" << yes_no_setting() << ")\n"; +} + +void wideStringCharsAt(int *v) { + // This function has "des" and "rsa" in the name. + COMPUTE(v) +} + +void bitcastVariable(int *v) { + // This function has "aria" and "cast" in the name. + COMPUTE(v) +} + +void dividesVariance(int *v) { + // This function has "des" and "aria" in the name. + COMPUTE(v) +} + +void broadcastNodes(int *v) { + // This function has "cast" and "des" in the name. + COMPUTE(v) +} + +#define ROTATE(val, amount) ( (val << amount) | (val >> (32 - amount)) ) + +static inline void hashMix(const int *data, int &state) { + // This function looks like part of a hashing function. It's not necessarily intended to + // be a cryptographic hash, so should not be flagged. + state ^= data[0]; + ROTATE(state, 1); + state ^= data[1]; + ROTATE(state, 7); + state ^= data[2]; + ROTATE(state, 11); + state ^= data[3]; + ROTATE(state, 3); + state ^= data[4]; + ROTATE(state, 13); + state ^= data[5]; + ROTATE(state, 5); + state ^= data[6]; + ROTATE(state, 2); + state ^= data[7]; + ROTATE(state, 17); +}