Merge pull request #5896 from geoffw0/weak_crypto

C++: Improve cpp/weak-cryptographic-algorithm
This commit is contained in:
Mathias Vorreiter Pedersen
2021-05-19 13:17:13 +02:00
committed by GitHub
7 changed files with 557 additions and 46 deletions

View File

@@ -0,0 +1,2 @@
lgtm,codescanning
* The "Use of a broken or risky cryptographic algorithm" (`cpp/weak-cryptographic-algorithm`) query has been enhanced to reduce false positive results, and (rarely) find more true positive results.

View File

@@ -13,39 +13,111 @@
import cpp
import semmle.code.cpp.security.Encryption
abstract class InsecureCryptoSpec extends Locatable {
abstract string description();
}
Function getAnInsecureFunction() {
result.getName().regexpMatch(getInsecureAlgorithmRegex()) and
/**
* A function which may relate to an insecure encryption algorithm.
*/
Function getAnInsecureEncryptionFunction() {
(
isInsecureEncryption(result.getName()) or
isInsecureEncryption(result.getAParameter().getName()) or
isInsecureEncryption(result.getDeclaringType().getName())
) and
exists(result.getACallToThisFunction())
}
class InsecureFunctionCall extends InsecureCryptoSpec, FunctionCall {
InsecureFunctionCall() { this.getTarget() = getAnInsecureFunction() }
override string description() { result = "function call" }
override string toString() { result = FunctionCall.super.toString() }
override Location getLocation() { result = FunctionCall.super.getLocation() }
/**
* A function with additional evidence it is related to encryption.
*/
Function getAdditionalEvidenceFunction() {
(
isEncryptionAdditionalEvidence(result.getName()) or
isEncryptionAdditionalEvidence(result.getAParameter().getName())
) and
exists(result.getACallToThisFunction())
}
Macro getAnInsecureMacro() {
result.getName().regexpMatch(getInsecureAlgorithmRegex()) and
/**
* A macro which may relate to an insecure encryption algorithm.
*/
Macro getAnInsecureEncryptionMacro() {
isInsecureEncryption(result.getName()) and
exists(result.getAnInvocation())
}
class InsecureMacroSpec extends InsecureCryptoSpec, MacroInvocation {
InsecureMacroSpec() { this.getMacro() = getAnInsecureMacro() }
override string description() { result = "macro invocation" }
override string toString() { result = MacroInvocation.super.toString() }
override Location getLocation() { result = MacroInvocation.super.getLocation() }
/**
* A macro with additional evidence it is related to encryption.
*/
Macro getAdditionalEvidenceMacro() {
isEncryptionAdditionalEvidence(result.getName()) and
exists(result.getAnInvocation())
}
from InsecureCryptoSpec c
select c, "This " + c.description() + " specifies a broken or weak cryptographic algorithm."
/**
* An enum constant which may relate to an insecure encryption algorithm.
*/
EnumConstant getAnInsecureEncryptionEnumConst() { isInsecureEncryption(result.getName()) }
/**
* An enum constant with additional evidence it is related to encryption.
*/
EnumConstant getAdditionalEvidenceEnumConst() { isEncryptionAdditionalEvidence(result.getName()) }
/**
* A function call we have a high confidence is related to use of an insecure
* encryption algorithm.
*/
class InsecureFunctionCall extends FunctionCall {
Element blame;
string explain;
InsecureFunctionCall() {
// find use of an insecure algorithm name
(
getTarget() = getAnInsecureEncryptionFunction() and
blame = this and
explain = "function call"
or
exists(MacroInvocation mi |
(
mi.getAnExpandedElement() = this or
mi.getAnExpandedElement() = this.getAnArgument()
) and
mi.getMacro() = getAnInsecureEncryptionMacro() and
blame = mi and
explain = "macro invocation"
)
or
exists(EnumConstantAccess ec |
ec = this.getAnArgument() and
ec.getTarget() = getAnInsecureEncryptionEnumConst() and
blame = ec and
explain = "enum constant access"
)
) and
// find additional evidence that this function is related to encryption.
(
getTarget() = getAdditionalEvidenceFunction()
or
exists(MacroInvocation mi |
(
mi.getAnExpandedElement() = this or
mi.getAnExpandedElement() = this.getAnArgument()
) and
mi.getMacro() = getAdditionalEvidenceMacro()
)
or
exists(EnumConstantAccess ec |
ec = this.getAnArgument() and
ec.getTarget() = getAdditionalEvidenceEnumConst()
)
)
}
Element getBlame() { result = blame }
string getDescription() { result = explain }
}
from InsecureFunctionCall c
select c.getBlame(),
"This " + c.getDescription() + " specifies a broken or weak cryptographic algorithm."

View File

@@ -10,10 +10,18 @@ import cpp
string getAnInsecureAlgorithmName() {
result =
[
"DES", "RC2", "RC4", "RC5", "ARCFOUR" // ARCFOUR is a variant of RC4
"DES", "RC2", "RC4", "RC5", "ARCFOUR", // ARCFOUR is a variant of RC4
"3DES", "DES3" // also appears separated, e.g. "TRIPLE-DES", which will be matched as "DES".
]
}
/**
* Gets the name of an algorithm that is known to be secure.
*/
string getASecureAlgorithmName() {
result = ["RSA", "SHA256", "CCM", "GCM", "AES", "Blowfish", "ECIES"]
}
/**
* Gets the name of a hash algorithm that is insecure if it is being used for
* encryption (but it is hard to know when that is happening).
@@ -23,25 +31,40 @@ string getAnInsecureHashAlgorithmName() { result = ["SHA1", "MD5"] }
/**
* Gets the regular expression used for matching strings that look like they
* contain an algorithm that is known to be insecure.
*
* Consider using `isInsecureEncryption` rather than accessing this regular
* expression directly.
*/
string getInsecureAlgorithmRegex() {
result =
// algorithms usually appear in names surrounded by characters that are not
// alphabetical characters in the same case. This handles the upper and lower
// case cases
"(^|.*[^A-Z])(" + strictconcat(getAnInsecureAlgorithmName(), "|") + ")([^A-Z].*|$)" + "|" +
// for lowercase, we want to be careful to avoid being confused by camelCase
// hence we require two preceding uppercase letters to be sure of a case switch,
// or a preceding non-alphabetic character
"(^|.*[A-Z]{2}|.*[^a-zA-Z])(" + strictconcat(getAnInsecureAlgorithmName().toLowerCase(), "|") +
")([^a-z].*|$)"
// alphabetical characters in the same case or numerical digits. This
// handles the upper case:
"(^|.*[^A-Z0-9])(" + strictconcat(getAnInsecureAlgorithmName(), "|") + ")([^A-Z0-9].*|$)" + "|" +
// for lowercase, we want to be careful to avoid being confused by
//camelCase, hence we require two preceding uppercase letters to be
// sure of a case switch (or a preceding non-alphabetic, non-numeric
// character).
"(^|.*[A-Z]{2}|.*[^a-zA-Z0-9])(" +
strictconcat(getAnInsecureAlgorithmName().toLowerCase(), "|") + ")([^a-z0-9].*|$)"
}
/**
* Gets the name of an algorithm that is known to be secure.
* Holds if `name` looks like it might be related to operations with an
* insecure encyption algorithm.
*/
string getASecureAlgorithmName() {
result = ["RSA", "SHA256", "CCM", "GCM", "AES", "Blowfish", "ECIES"]
bindingset[name]
predicate isInsecureEncryption(string name) { name.regexpMatch(getInsecureAlgorithmRegex()) }
/**
* Holds if there is additional evidence that `name` looks like it might be
* related to operations with an encyption algorithm, besides the name of a
* specific algorithm. This can be used in conjuction with
* `isInsecureEncryption` to produce a stronger heuristic.
*/
bindingset[name]
predicate isEncryptionAdditionalEvidence(string name) {
name.toUpperCase().matches("%" + ["CRYPT", "CODE", "CODING", "CBC", "KEY", "CIPHER", "MAC"] + "%")
}
/**
@@ -51,14 +74,15 @@ string getASecureAlgorithmName() {
string getSecureAlgorithmRegex() {
result =
// algorithms usually appear in names surrounded by characters that are not
// alphabetical characters in the same case. This handles the upper and lower
// case cases
"(^|.*[^A-Z])(" + strictconcat(getASecureAlgorithmName(), "|") + ")([^A-Z].*|$)" + "|" +
// for lowercase, we want to be careful to avoid being confused by camelCase
// hence we require two preceding uppercase letters to be sure of a case
// switch, or a preceding non-alphabetic character
"(^|.*[A-Z]{2}|.*[^a-zA-Z])(" + strictconcat(getASecureAlgorithmName().toLowerCase(), "|") +
")([^a-z].*|$)"
// alphabetical characters in the same case or numerical digits. This
// handles the upper case:
"(^|.*[^A-Z0-9])(" + strictconcat(getASecureAlgorithmName(), "|") + ")([^A-Z0-9].*|$)" + "|" +
// for lowercase, we want to be careful to avoid being confused by
//camelCase, hence we require two preceding uppercase letters to be
// sure of a case switch (or a preceding non-alphabetic, non-numeric
// character).
"(^|.*[A-Z]{2}|.*[^a-zA-Z0-9])(" + strictconcat(getASecureAlgorithmName().toLowerCase(), "|") +
")([^a-z0-9].*|$)"
}
/**

View File

@@ -0,0 +1,25 @@
| test2.cpp:49:4:49:24 | call to my_des_implementation | This function call specifies a broken or weak cryptographic algorithm. |
| test2.cpp:62:33:62:40 | ALGO_DES | This macro invocation specifies a broken or weak cryptographic algorithm. |
| test2.cpp:124:4:124:24 | call to my_des_implementation | This function call specifies a broken or weak cryptographic algorithm. |
| test2.cpp:144:27:144:29 | DES | This enum constant access specifies a broken or weak cryptographic algorithm. |
| test2.cpp:172:28:172:35 | ALGO_DES | This macro invocation specifies a broken or weak cryptographic algorithm. |
| test2.cpp:175:28:175:34 | USE_DES | This enum constant access specifies a broken or weak cryptographic algorithm. |
| test2.cpp:182:38:182:45 | ALGO_DES | This macro invocation specifies a broken or weak cryptographic algorithm. |
| test2.cpp:185:38:185:44 | USE_DES | This enum constant access specifies a broken or weak cryptographic algorithm. |
| test2.cpp:238:2:238:20 | call to encrypt | This function call specifies a broken or weak cryptographic algorithm. |
| test2.cpp:245:5:245:11 | call to encrypt | This function call specifies a broken or weak cryptographic algorithm. |
| test.cpp:38:2:38:31 | ENCRYPT_WITH_DES(data,amount) | This macro invocation specifies a broken or weak cryptographic algorithm. |
| test.cpp:39:2:39:31 | ENCRYPT_WITH_RC2(data,amount) | This macro invocation specifies a broken or weak cryptographic algorithm. |
| test.cpp:41:2:41:32 | ENCRYPT_WITH_3DES(data,amount) | This macro invocation specifies a broken or weak cryptographic algorithm. |
| test.cpp:42:2:42:38 | ENCRYPT_WITH_TRIPLE_DES(data,amount) | This macro invocation specifies a broken or weak cryptographic algorithm. |
| test.cpp:51:2:51:32 | DES_DO_ENCRYPTION(data,amount) | This macro invocation specifies a broken or weak cryptographic algorithm. |
| test.cpp:52:2:52:31 | RUN_DES_ENCODING(data,amount) | This macro invocation specifies a broken or weak cryptographic algorithm. |
| test.cpp:53:2:53:25 | DES_ENCODE(data,amount) | This macro invocation specifies a broken or weak cryptographic algorithm. |
| test.cpp:54:2:54:26 | DES_SET_KEY(data,amount) | This macro invocation specifies a broken or weak cryptographic algorithm. |
| test.cpp:88:2:88:11 | call to encryptDES | This function call specifies a broken or weak cryptographic algorithm. |
| test.cpp:89:2:89:11 | call to encryptRC2 | This function call specifies a broken or weak cryptographic algorithm. |
| test.cpp:91:2:91:12 | call to encrypt3DES | This function call specifies a broken or weak cryptographic algorithm. |
| test.cpp:92:2:92:17 | call to encryptTripleDES | This function call specifies a broken or weak cryptographic algorithm. |
| test.cpp:101:2:101:15 | call to do_des_encrypt | This function call specifies a broken or weak cryptographic algorithm. |
| test.cpp:102:2:102:12 | call to DES_Set_Key | This function call specifies a broken or weak cryptographic algorithm. |
| test.cpp:121:2:121:24 | INIT_ENCRYPT_WITH_DES() | This macro invocation specifies a broken or weak cryptographic algorithm. |

View File

@@ -0,0 +1 @@
Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql

View File

@@ -0,0 +1,125 @@
typedef unsigned long size_t;
// --- simple encryption macro invocations ---
void my_implementation1(void *data, size_t amount);
void my_implementation2(void *data, size_t amount);
void my_implementation3(void *data, size_t amount);
void my_implementation4(void *data, size_t amount);
void my_implementation5(void *data, size_t amount);
void my_implementation6(const char *str);
#define ENCRYPT_WITH_DES(data, amount) my_implementation1(data, amount)
#define ENCRYPT_WITH_RC2(data, amount) my_implementation2(data, amount)
#define ENCRYPT_WITH_AES(data, amount) my_implementation3(data, amount)
#define ENCRYPT_WITH_3DES(data, amount) my_implementation4(data, amount)
#define ENCRYPT_WITH_TRIPLE_DES(data, amount) my_implementation4(data, amount)
#define ENCRYPT_WITH_RC20(data, amount) my_implementation5(data, amount)
#define ENCRYPT_WITH_DES_REMOVED(data, amount)
#define DESENCRYPT(data, amount) my_implementation1(data, amount)
#define RC2ENCRYPT(data, amount) my_implementation2(data, amount)
#define AESENCRYPT(data, amount) my_implementation3(data, amount)
#define DES3ENCRYPT(data, amount) my_implementation4(data, amount)
#define DES_DO_ENCRYPTION(data, amount) my_implementation1(data, amount)
#define RUN_DES_ENCODING(data, amount) my_implementation1(data, amount)
#define DES_ENCODE(data, amount) my_implementation1(data, amount)
#define DES_SET_KEY(data, amount) my_implementation1(data, amount)
#define DES(str) my_implementation6(str)
#define DESMOND(str) my_implementation6(str)
#define ANODES(str) my_implementation6(str)
#define SORT_ORDER_DES (1)
void test_macros(void *data, size_t amount, const char *str)
{
ENCRYPT_WITH_DES(data, amount); // BAD
ENCRYPT_WITH_RC2(data, amount); // BAD
ENCRYPT_WITH_AES(data, amount); // GOOD (good algorithm)
ENCRYPT_WITH_3DES(data, amount); // BAD
ENCRYPT_WITH_TRIPLE_DES(data, amount); // BAD
ENCRYPT_WITH_RC20(data, amount); // GOOD (if there ever is an RC20 algorithm, we have no reason to believe it's weak)
ENCRYPT_WITH_DES_REMOVED(data, amount); // GOOD (implementation has been deleted)
DESENCRYPT(data, amount); // BAD [NOT DETECTED]
RC2ENCRYPT(data, amount); // BAD [NOT DETECTED]
AESENCRYPT(data, amount); // GOOD (good algorithm)
DES3ENCRYPT(data, amount); // BAD [NOT DETECTED]
DES_DO_ENCRYPTION(data, amount); // BAD
RUN_DES_ENCODING(data, amount); // BAD
DES_ENCODE(data, amount); // BAD
DES_SET_KEY(data, amount); // BAD
DES(str); // GOOD (probably nothing to do with encryption)
DESMOND(str); // GOOD (probably nothing to do with encryption)
ANODES(str); // GOOD (probably nothing to do with encryption)
int ord = SORT_ORDER_DES; // GOOD (probably nothing to do with encryption)
}
// --- simple encryption function calls ---
void encryptDES(void *data, size_t amount);
void encryptRC2(void *data, size_t amount);
void encryptAES(void *data, size_t amount);
void encrypt3DES(void *data, size_t amount);
void encryptTripleDES(void *data, size_t amount);
void DESEncrypt(void *data, size_t amount);
void RC2Encrypt(void *data, size_t amount);
void AESEncrypt(void *data, size_t amount);
void DES3Encrypt(void *data, size_t amount);
void DoDESEncryption(void *data, size_t amount);
void encryptDes(void *data, size_t amount);
void do_des_encrypt(void *data, size_t amount);
void DES_Set_Key(const char *key);
void DESSetKey(const char *key);
int Des();
void Desmond(const char *str);
void Anodes(int i);
void ConDes();
void test_functions(void *data, size_t amount, const char *str)
{
encryptDES(data, amount); // BAD
encryptRC2(data, amount); // BAD
encryptAES(data, amount); // GOOD (good algorithm)
encrypt3DES(data, amount); // BAD
encryptTripleDES(data, amount); // BAD
DESEncrypt(data, amount); // BAD [NOT DETECTED]
RC2Encrypt(data, amount); // BAD [NOT DETECTED]
AESEncrypt(data, amount); // GOOD (good algorithm)
DES3Encrypt(data, amount); // BAD [NOT DETECTED]
DoDESEncryption(data, amount); // BAD [NOT DETECTED]
encryptDes(data, amount); // BAD [NOT DETECTED]
do_des_encrypt(data, amount); // BAD
DES_Set_Key(str); // BAD
DESSetKey(str); // BAD [NOT DETECTED]
Des(); // GOOD (probably nothing to do with encryption)
Desmond(str); // GOOD (probably nothing to do with encryption)
Anodes(1); // GOOD (probably nothing to do with encryption)
ConDes(); // GOOD (probably nothing to do with encryption)
}
// --- macros for functions with no arguments ---
void my_implementation7();
void my_implementation8();
#define INIT_ENCRYPT_WITH_DES() my_implementation7()
#define INIT_ENCRYPT_WITH_AES() my_implementation8()
void test_macros2()
{
INIT_ENCRYPT_WITH_DES(); // BAD
INIT_ENCRYPT_WITH_AES(); // GOOD (good algorithm)
// ...
}

View File

@@ -0,0 +1,262 @@
typedef unsigned long size_t;
int strcmp(const char *s1, const char *s2);
void abort(void);
struct keytype
{
char data[16];
};
void my_des_implementation(char *data, size_t amount, keytype key);
void my_rc2_implementation(char *data, size_t amount, keytype key);
void my_aes_implementation(char *data, size_t amount, keytype key);
void my_3des_implementation(char *data, size_t amount, keytype key);
typedef void (*implementation_fn_ptr)(char *data, size_t amount, keytype key);
// --- more involved C-style example ---
#define ALGO_DES (1)
#define ALGO_AES (2)
int all_algos[] = {
ALGO_DES,
ALGO_AES
};
void encrypt_good(char *data, size_t amount, keytype key, int algo)
{
switch (algo)
{
case ALGO_DES:
abort();
case ALGO_AES:
{
my_aes_implementation(data, amount, key); // GOOD
} break;
}
};
void encrypt_bad(char *data, size_t amount, keytype key, int algo)
{
switch (algo)
{
case ALGO_DES:
{
my_des_implementation(data, amount, key); // BAD
} break;
case ALGO_AES:
{
my_aes_implementation(data, amount, key); // GOOD
} break;
}
};
void do_encrypts(char *data, size_t amount, keytype key)
{
encrypt_good(data, amount, key, ALGO_AES); // GOOD
encrypt_bad(data, amount, key, ALGO_DES); // BAD
}
// --- more involved CPP-style example ---
enum algorithm
{
DES,
AES
};
algorithm all_algorithms[] = {
DES,
AES
};
class MyGoodEncryptor
{
public:
MyGoodEncryptor(keytype _key, algorithm _algo) : key(_key), algo(_algo) {};
void encrypt(char *data, size_t amount);
private:
keytype key;
algorithm algo;
};
void MyGoodEncryptor :: encrypt(char *data, size_t amount)
{
switch (algo)
{
case DES:
{
throw "DES is not a good choice of encryption algorithm!";
} break;
case AES:
{
my_aes_implementation(data, amount, key); // GOOD
} break;
}
}
class MyBadEncryptor
{
public:
MyBadEncryptor(keytype _key, algorithm _algo) : key(_key), algo(_algo) {};
void encrypt(char *data, size_t amount);
private:
keytype key;
algorithm algo;
};
void MyBadEncryptor :: encrypt(char *data, size_t amount)
{
switch (algo)
{
case DES:
{
my_des_implementation(data, amount, key); // BAD
} break;
case AES:
{
my_aes_implementation(data, amount, key); // GOOD
} break;
}
}
void do_class_encrypts(char *data, size_t amount, keytype key)
{
{
MyGoodEncryptor mge(key, AES); // GOOD
mge.encrypt(data, amount);
}
{
MyBadEncryptor mbe(key, DES); // BAD
mbe.encrypt(data, amount);
}
}
// --- unseen implementation ---
enum use_algorithm
{
USE_DES,
USE_AES
};
void set_encryption_algorithm1(int algorithm);
void set_encryption_algorithm2(use_algorithm algorithm);
void set_encryption_algorithm3(const char *algorithm_str);
void encryption_with1(char *data, size_t amount, keytype key, int algorithm);
void encryption_with2(char *data, size_t amount, keytype key, use_algorithm algorithm);
void encryption_with3(char *data, size_t amount, keytype key, const char *algorithm_str);
int get_algorithm1();
use_algorithm get_algorithm2();
const char *get_algorithm3();
void do_unseen_encrypts(char *data, size_t amount, keytype key)
{
set_encryption_algorithm1(ALGO_DES); // BAD
set_encryption_algorithm1(ALGO_AES); // GOOD
set_encryption_algorithm2(USE_DES); // BAD
set_encryption_algorithm2(USE_AES); // GOOD
set_encryption_algorithm3("DES"); // BAD [NOT DETECTED]
set_encryption_algorithm3("AES"); // GOOD
set_encryption_algorithm3("AES-256"); // GOOD
encryption_with1(data, amount, key, ALGO_DES); // BAD
encryption_with1(data, amount, key, ALGO_AES); // GOOD
encryption_with2(data, amount, key, USE_DES); // BAD
encryption_with2(data, amount, key, USE_AES); // GOOD
encryption_with3(data, amount, key, "DES"); // BAD [NOT DETECTED]
encryption_with3(data, amount, key, "AES"); // GOOD
encryption_with3(data, amount, key, "AES-256"); // GOOD
if (get_algorithm1() == ALGO_DES) // GOOD
{
throw "DES is not a good choice of encryption algorithm!";
}
if (get_algorithm2() == USE_DES) // GOOD
{
throw "DES is not a good choice of encryption algorithm!";
}
if (strcmp(get_algorithm3(), "DES") == 0) // GOOD
{
throw "DES is not a good choice of encryption algorithm!";
}
}
// --- classes ---
class desEncrypt
{
public:
static void encrypt(const char *data);
static void doSomethingElse();
};
class aes256Encrypt
{
public:
static void encrypt(const char *data);
static void doSomethingElse();
};
class desCipher
{
public:
void encrypt(const char *data);
void doSomethingElse();
};
class aesCipher
{
public:
void encrypt(const char *data);
void doSomethingElse();
};
void do_classes(const char *data)
{
desEncrypt::encrypt(data); // BAD
aes256Encrypt::encrypt(data); // GOOD
desEncrypt::doSomethingElse(); // GOOD
aes256Encrypt::doSomethingElse(); // GOOD
desCipher dc;
aesCipher ac;
dc.encrypt(data); // BAD
ac.encrypt(data); // GOOD
dc.doSomethingElse(); // GOOD
ac.doSomethingElse(); // GOOD
}
// --- function pointer ---
void do_fn_ptr(char *data, size_t amount, keytype key)
{
implementation_fn_ptr impl;
impl = &my_des_implementation; // BAD [NOT DETECTED]
impl(data, amount, key);
impl = &my_aes_implementation; // GOOD
impl(data, amount, key);
}