From b0514de0944b8a9686a229f5b362999e3ddf112b Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 29 Nov 2023 16:31:06 +0000 Subject: [PATCH 1/9] C++: Add cpp/crypto-primitive query to experimental. --- .../CWE-1240/CustomCryptographicPrimitive.ql | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql 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..ffdf16e6e26 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql @@ -0,0 +1,85 @@ +/** + * @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 + * external/cwe/cwe-1240 + */ + +import cpp + +/** + * A word that might be in the name of an encryption function. + */ +string encryptionWord() { + exists(string word | + // `(?= 8 and + exists(f.getFile().getRelativePath()) // exclude library files +select f, "This may be a custom implementation of a cryptographic primitive." From 3c6f318cb246d0b4ba0e9ee6799b29eaf64eb212 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 29 Nov 2023 16:34:00 +0000 Subject: [PATCH 2/9] C++: Add query tests. --- .../CustomCryptographicPrimitive.expected | 3 + .../CustomCryptographicPrimitive.qlref | 1 + .../Security/CWE/CWE-1240/tests_crypto.cpp | 97 ++++++++++++++++ .../CWE/CWE-1240/tests_not_crypto.cpp | 107 ++++++++++++++++++ 4 files changed, 208 insertions(+) create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/CustomCryptographicPrimitive.expected create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/CustomCryptographicPrimitive.qlref create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/tests_crypto.cpp create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/tests_not_crypto.cpp 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..fba9ba71990 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/CustomCryptographicPrimitive.expected @@ -0,0 +1,3 @@ +| tests_crypto.cpp:11:6:11:18 | encryptString | This may be a custom implementation of a cryptographic primitive. | +| tests_crypto.cpp:30:6:30:14 | MyEncrypt | This may be a custom implementation of a cryptographic primitive. | +| tests_crypto.cpp:83:6:83:18 | init_aes_sbox | This 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/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..a026782d5d2 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/tests_not_crypto.cpp @@ -0,0 +1,107 @@ +// 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; +} + +// --- + +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"; +} + +// 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]; + +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) +} From c83cfe49361c79bf3a7187d18f83e01e45a410cb Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 30 Nov 2023 14:56:13 +0000 Subject: [PATCH 3/9] C++: Make output clearer in cases where the function name is a macro expansion (I've seen this more than once). --- .../Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql | 4 +++- .../CWE/CWE-1240/CustomCryptographicPrimitive.expected | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql b/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql index ffdf16e6e26..ac0b65d996e 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql @@ -82,4 +82,6 @@ where amount = strictcount(Expr e | computeHeuristic(e) and e.getEnclosingFunction() = f) and amount >= 8 and exists(f.getFile().getRelativePath()) // exclude library files -select f, "This may be a custom implementation of a cryptographic primitive." +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 index fba9ba71990..500d5be0a69 100644 --- 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 @@ -1,3 +1,3 @@ -| tests_crypto.cpp:11:6:11:18 | encryptString | This may be a custom implementation of a cryptographic primitive. | -| tests_crypto.cpp:30:6:30:14 | MyEncrypt | This may be a custom implementation of a cryptographic primitive. | -| tests_crypto.cpp:83:6:83:18 | init_aes_sbox | This may be a custom implementation of a cryptographic primitive. | +| 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:83:6:83:18 | init_aes_sbox | This function, "init_aes_sbox", may be a custom implementation of a cryptographic primitive. | From fb02e996d4c79e6f1f1ec65fd570613d99381086 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 30 Nov 2023 16:03:13 +0000 Subject: [PATCH 4/9] C++: Address QL-for-QL comments. --- .../CWE-1240/CustomCryptographicPrimitive.ql | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql b/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql index ac0b65d996e..a1e6bc5a019 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql @@ -13,11 +13,11 @@ import cpp /** - * A word that might be in the name of an encryption function. + * Gets a word that might be in the name of an encryption function. */ string encryptionWord() { exists(string word | - // `(? Date: Fri, 1 Dec 2023 19:01:01 +0000 Subject: [PATCH 5/9] C++: Add more test cases. --- .../CustomCryptographicPrimitive.expected | 1 + .../CWE/CWE-1240/library/tests_library.h | 6 +++++ .../CWE/CWE-1240/tests_not_crypto.cpp | 22 +++++++++++++------ 3 files changed, 22 insertions(+), 7 deletions(-) create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/library/tests_library.h 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 index 500d5be0a69..c057a0d022c 100644 --- 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 @@ -1,3 +1,4 @@ +| library/tests_library.h:4:6:4:19 | do_aes_encrypt | This function, "do_aes_encrypt", may be a custom implementation of a cryptographic primitive. | | 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: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/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_not_crypto.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-1240/tests_not_crypto.cpp index a026782d5d2..d902c8090b5 100644 --- 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 @@ -21,8 +21,23 @@ namespace std 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) { @@ -79,13 +94,6 @@ void output_encrypt_decrypt_algorithms() { std::cout << indent << "Chacha (" << yes_no_setting() << ")\n"; } -// 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]; - void wideStringCharsAt(int *v) { // This function has "des" and "rsa" in the name. COMPUTE(v) From 2f0be40f3797ef000d1a4c324a9703b9ce7fe244 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 1 Dec 2023 17:30:21 +0000 Subject: [PATCH 6/9] C++: Exclude results in some common libraries. --- .../CWE-1240/CustomCryptographicPrimitive.ql | 24 ++++++++++++++++++- .../CustomCryptographicPrimitive.expected | 1 - 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql b/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql index a1e6bc5a019..6cfd6256a0f 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql @@ -76,12 +76,34 @@ predicate computeHeuristic(Expr e) { computeHeuristicType(e.getUnspecifiedType()) } +/** + * Gets the name of an established cryptography library or likely third party directory. + */ +string encryptionLibraryName() { + result = + [ + "libssh", "openssl", "boringssl", "mbed", "libsodium", "libsrtp", "third.?party", "library", + "deps" + ] +} + +/** + * Holds if `f` is a file that is likely to be inside an established + * cryptography library. + */ +predicate isLibrary(File f) { + f.getAbsolutePath().regexpMatch("(?i).*(" + concat(encryptionLibraryName(), "|") + ").*") + or + // assume that any result that would be found outside the source location is in a crypto library + not exists(f.getFile().getRelativePath()) +} + from Function f, int amount where likelyEncryptionFunction(f) and amount = strictcount(Expr e | computeHeuristic(e) and e.getEnclosingFunction() = f) and amount >= 8 and - exists(f.getFile().getRelativePath()) // exclude library files + 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 index c057a0d022c..500d5be0a69 100644 --- 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 @@ -1,4 +1,3 @@ -| library/tests_library.h:4:6:4:19 | do_aes_encrypt | This function, "do_aes_encrypt", may be a custom implementation of a cryptographic primitive. | | 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:83:6:83:18 | init_aes_sbox | This function, "init_aes_sbox", may be a custom implementation of a cryptographic primitive. | From cde975dc2464b9a5a6648b671a1f4ecb7c0d1d95 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Mon, 4 Dec 2023 11:36:49 +0000 Subject: [PATCH 7/9] C++: Add even more test cases. --- .../CWE/CWE-1240/tests_not_crypto.cpp | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) 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 index d902c8090b5..c84d4dbba77 100644 --- 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 @@ -113,3 +113,26 @@ 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); +} From e95098f61f12a5e3f2d0a3405a1b4ea203dadb0d Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Mon, 4 Dec 2023 12:11:28 +0000 Subject: [PATCH 8/9] C++: Add 'experimental' tag. --- .../Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql b/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql index 6cfd6256a0f..e680aabdccf 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql @@ -7,6 +7,7 @@ * @precision medium * @id cpp/crypto-primitive * @tags security + * experimental * external/cwe/cwe-1240 */ From 521d98ed8d476acb6bfaae9161e99ac80c409f97 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 5 Dec 2023 17:34:42 +0000 Subject: [PATCH 9/9] C++: Make the encryption words a tiny bit more flexible. --- .../Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql | 2 +- .../Security/CWE/CWE-1240/CustomCryptographicPrimitive.expected | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql b/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql index e680aabdccf..751282e0f40 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-1240/CustomCryptographicPrimitive.ql @@ -29,7 +29,7 @@ string encryptionWord() { "Camellia", //"(?