C++: Exclude macro invocations in switch case expressions.

Author: geoffw0

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

@@ -45,7 +45,10 @@ class InsecureMacroSpec extends InsecureCryptoSpec, MacroInvocation {
     // the macro name suggests it relates to an insecure crypto algorithm.
     this.getMacro() = getAnInsecureMacro() and
     // the macro invocation generates something.
-    exists(this.getAGeneratedElement())
+    exists(this.getAGeneratedElement().(ControlFlowNode)) and
+    // exclude expressions controlling ifs/switches (as they may not be used).
+    not any(IfStmt c).getCondition().getAChild*() = this.getAGeneratedElement() and
+    not any(SwitchCase c).getExpr().getAChild*() = this.getAGeneratedElement()
   }
 
   override string description() { result = "macro invocation" }

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

@@ -1,12 +1,9 @@
 | test2.cpp:25:2:25:9 | ALGO_DES | This macro invocation specifies a broken or weak cryptographic algorithm. |
-| test2.cpp:33:7:33:14 | ALGO_DES | This macro invocation specifies a broken or weak cryptographic algorithm. |
-| test2.cpp:47:7:47:14 | ALGO_DES | This macro invocation specifies a broken or weak cryptographic algorithm. |
 | 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:172:28:172:35 | ALGO_DES | This macro invocation 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:192:26:192:33 | ALGO_DES | This macro invocation 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:42:2:42:38 | ENCRYPT_WITH_TRIPLE_DES(data,amount) | This macro invocation specifies a broken or weak cryptographic algorithm. |

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

@@ -30,7 +30,7 @@ void encrypt_good(char *data, size_t amount, keytype key, int algo)
 {
 	switch (algo)
 	{
-	case ALGO_DES: // [FALSE POSITIVE]
+	case ALGO_DES:
 		abort();
 
 	case ALGO_AES:
@@ -189,7 +189,7 @@ void do_unseen_encrypts(char *data, size_t amount, keytype key)
 	encryption_with3(data, amount, key, "AES"); // GOOD
 	encryption_with3(data, amount, key, "AES-256"); // GOOD
 
-	if (get_algorithm1() == ALGO_DES) // GOOD [FALSE POSITIVE]
+	if (get_algorithm1() == ALGO_DES) // GOOD
 	{
 		throw "DES is not a good choice of encryption algorithm!";
 	}