Skip to content

Commit e5147c2

Browse files
committed
C++: Exclude functions that don't involve buffers.
1 parent a481e5c commit e5147c2

File tree

4 files changed

+29
-21
lines changed

4 files changed

+29
-21
lines changed

cpp/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,25 @@ predicate getInsecureEncryptionEvidence(FunctionCall fc, Element blame, string d
110110
) and
111111
// exclude calls from templates as this is rarely the right place to flag an
112112
// issue
113-
not fc.isFromTemplateInstantiation(_)
113+
not fc.isFromTemplateInstantiation(_) and
114+
(
115+
// the function should have an input that looks like a non-constant buffer
116+
exists(Expr e |
117+
fc.getAnArgument() = e and
118+
(
119+
e.getUnspecifiedType() instanceof PointerType or
120+
e.getUnspecifiedType() instanceof ReferenceType or
121+
e.getUnspecifiedType() instanceof ArrayType
122+
) and
123+
not e.getType().isDeeplyConstBelow() and
124+
not e.isConstant()
125+
)
126+
or
127+
// or be a non-const member function of an object
128+
fc.getTarget() instanceof MemberFunction and
129+
not fc.getTarget() instanceof ConstMemberFunction and
130+
not fc.getTarget().isStatic()
131+
)
114132
}
115133

116134
/**

cpp/ql/test/query-tests/Security/CWE/CWE-327/BrokenCryptoAlgorithm.expected

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,13 @@
33
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:66:31:66:38 | ALGO_DES | invocation of macro ALGO_DES |
44
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:128:4:128:24 | call to my_des_implementation | call to my_des_implementation |
55
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:148:27:148:29 | DES | access of enum constant DES |
6-
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:176:28:176:35 | ALGO_DES | invocation of macro ALGO_DES |
7-
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:179:28:179:34 | USE_DES | access of enum constant USE_DES |
86
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:186:38:186:45 | ALGO_DES | invocation of macro ALGO_DES |
97
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:189:38:189:44 | USE_DES | access of enum constant USE_DES |
108
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:242:2:242:20 | call to encrypt | call to encrypt |
119
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:249:5:249:11 | call to encrypt | call to encrypt |
1210
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:304:20:304:37 | call to desEncryptor | call to desEncryptor |
1311
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:308:5:308:19 | call to doDesEncryption | call to doDesEncryption |
1412
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:309:9:309:23 | call to doDesEncryption | call to doDesEncryption |
15-
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:325:2:325:57 | ALGO_DES | invocation of macro ALGO_DES |
16-
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:340:24:340:42 | ENCRYPTION_DES_NAME | invocation of macro ENCRYPTION_DES_NAME |
17-
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:362:24:362:43 | call to getEncryptionNameDES | call to getEncryptionNameDES |
18-
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:377:10:377:29 | call to getEncryptionNameDES | call to getEncryptionNameDES |
19-
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:387:42:387:49 | ALGO_DES | invocation of macro ALGO_DES |
20-
| test2.cpp:49:4:49:24 | call to my_des_implementation | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test2.cpp:403:26:403:45 | call to getEncryptionNameDES | call to getEncryptionNameDES |
2113
| test.cpp:38:2:38:31 | ENCRYPT_WITH_DES(data,amount) | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test.cpp:38:2:38:31 | ENCRYPT_WITH_DES(data,amount) | invocation of macro ENCRYPT_WITH_DES |
2214
| test.cpp:38:2:38:31 | ENCRYPT_WITH_DES(data,amount) | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test.cpp:39:2:39:31 | ENCRYPT_WITH_RC2(data,amount) | invocation of macro ENCRYPT_WITH_RC2 |
2315
| test.cpp:38:2:38:31 | ENCRYPT_WITH_DES(data,amount) | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test.cpp:41:2:41:32 | ENCRYPT_WITH_3DES(data,amount) | invocation of macro ENCRYPT_WITH_3DES |
@@ -31,5 +23,3 @@
3123
| test.cpp:38:2:38:31 | ENCRYPT_WITH_DES(data,amount) | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test.cpp:91:2:91:12 | call to encrypt3DES | call to encrypt3DES |
3224
| test.cpp:38:2:38:31 | ENCRYPT_WITH_DES(data,amount) | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test.cpp:92:2:92:17 | call to encryptTripleDES | call to encryptTripleDES |
3325
| test.cpp:38:2:38:31 | ENCRYPT_WITH_DES(data,amount) | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test.cpp:101:2:101:15 | call to do_des_encrypt | call to do_des_encrypt |
34-
| test.cpp:38:2:38:31 | ENCRYPT_WITH_DES(data,amount) | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test.cpp:102:2:102:12 | call to DES_Set_Key | call to DES_Set_Key |
35-
| test.cpp:38:2:38:31 | ENCRYPT_WITH_DES(data,amount) | This file makes use of a broken or weak cryptographic algorithm (specified by $@). | test.cpp:121:2:121:24 | INIT_ENCRYPT_WITH_DES() | invocation of macro INIT_ENCRYPT_WITH_DES |

cpp/ql/test/query-tests/Security/CWE/CWE-327/test.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ void test_functions(void *data, size_t amount, const char *str)
9999
DoDESEncryption(data, amount); // BAD [NOT DETECTED]
100100
encryptDes(data, amount); // BAD [NOT DETECTED]
101101
do_des_encrypt(data, amount); // BAD
102-
DES_Set_Key(str); // BAD
102+
DES_Set_Key(str); // BAD [NOT DETECTED]
103103
DESSetKey(str); // BAD [NOT DETECTED]
104104

105105
Des(); // GOOD (probably nothing to do with encryption)
@@ -118,7 +118,7 @@ void my_implementation8();
118118

119119
void test_macros2()
120120
{
121-
INIT_ENCRYPT_WITH_DES(); // BAD
121+
INIT_ENCRYPT_WITH_DES(); // BAD [NOT DETECTED]
122122
INIT_ENCRYPT_WITH_AES(); // GOOD (good algorithm)
123123

124124
// ...

cpp/ql/test/query-tests/Security/CWE/CWE-327/test2.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -173,10 +173,10 @@ const char *get_algorithm3();
173173

174174
void do_unseen_encrypts(char *data, size_t amount, keytype key)
175175
{
176-
set_encryption_algorithm1(ALGO_DES); // BAD
176+
set_encryption_algorithm1(ALGO_DES); // BAD [NOT DETECTED]
177177
set_encryption_algorithm1(ALGO_AES); // GOOD
178178

179-
set_encryption_algorithm2(USE_DES); // BAD
179+
set_encryption_algorithm2(USE_DES); // BAD [NOT DETECTED]
180180
set_encryption_algorithm2(USE_AES); // GOOD
181181

182182
set_encryption_algorithm3("DES"); // BAD [NOT DETECTED]
@@ -322,7 +322,7 @@ const algorithmInfo *getEncryptionAlgorithmInfo(int algo);
322322
void test_assert(int algo, algorithmInfo *algoInfo)
323323
{
324324
assert(algo != ALGO_DES); // GOOD
325-
assert(algoInfo != getEncryptionAlgorithmInfo(ALGO_DES)); // GOOD [FALSE POSITIVE]
325+
assert(algoInfo != getEncryptionAlgorithmInfo(ALGO_DES)); // GOOD
326326

327327
// ...
328328
}
@@ -337,7 +337,7 @@ void abort(void);
337337

338338
void test_string_comparisons1(const char *algo_name)
339339
{
340-
if (strcmp(algo_name, ENCRYPTION_DES_NAME) == 0) // GOOD [FALSE POSITIVE]
340+
if (strcmp(algo_name, ENCRYPTION_DES_NAME) == 0) // GOOD
341341
{
342342
abort();
343343
}
@@ -359,7 +359,7 @@ const char *getEncryptionNameAES()
359359

360360
void test_string_comparisons2(const char *algo_name)
361361
{
362-
if (strcmp(algo_name, getEncryptionNameDES()) == 0) // GOOD [FALSE POSITIVE]
362+
if (strcmp(algo_name, getEncryptionNameDES()) == 0) // GOOD
363363
{
364364
abort();
365365
}
@@ -374,7 +374,7 @@ const char *getEncryptionName(int algo)
374374
switch (algo)
375375
{
376376
case ALGO_DES:
377-
return getEncryptionNameDES(); // GOOD [FALSE POSITIVE]
377+
return getEncryptionNameDES(); // GOOD
378378
case ALGO_AES:
379379
return getEncryptionNameAES(); // GOOD
380380
default:
@@ -384,7 +384,7 @@ const char *getEncryptionName(int algo)
384384

385385
void test_string_comparisons3(const char *algo_name)
386386
{
387-
if (strcmp(algo_name, getEncryptionName(ALGO_DES)) == 0) // GOOD [FALSE POSITIVE]
387+
if (strcmp(algo_name, getEncryptionName(ALGO_DES)) == 0) // GOOD
388388
{
389389
abort();
390390
}
@@ -400,6 +400,6 @@ void doEncryption(char *data, size_t len, const char *algorithmName);
400400

401401
void test_fn_in_fn(char *data, size_t len)
402402
{
403-
doEncryption(data, len, getEncryptionNameDES()); // BAD
403+
doEncryption(data, len, getEncryptionNameDES()); // BAD [NOT DETECTED]
404404
doEncryption(data, len, getEncryptionNameAES()); // GOOD
405405
}

0 commit comments

Comments
 (0)