From b24dc810c9fd64c57bdf9aa6eba3a92f57f69443 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 3 Jun 2021 20:21:08 +0100 Subject: [PATCH] C++: Combine results from cpp/weak-cryptographic-algorithm that are in the same file. --- .../CWE/CWE-327/BrokenCryptoAlgorithm.ql | 123 ++++++++++-------- .../CWE-327/BrokenCryptoAlgorithm.expected | 50 +++---- 2 files changed, 95 insertions(+), 78 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql b/cpp/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql index 7162b000dfc2..24f0b216d09d 100644 --- a/cpp/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql +++ b/cpp/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql @@ -28,7 +28,7 @@ Function getAnInsecureEncryptionFunction() { /** * A function with additional evidence it is related to encryption. */ -Function getAdditionalEvidenceFunction() { +Function getAnAdditionalEvidenceFunction() { ( isEncryptionAdditionalEvidence(result.getName()) or isEncryptionAdditionalEvidence(result.getAParameter().getName()) @@ -47,7 +47,7 @@ Macro getAnInsecureEncryptionMacro() { /** * A macro with additional evidence it is related to encryption. */ -Macro getAdditionalEvidenceMacro() { +Macro getAnAdditionalEvidenceMacro() { isEncryptionAdditionalEvidence(result.getName()) and exists(result.getAnInvocation()) } @@ -63,61 +63,78 @@ EnumConstant getAnInsecureEncryptionEnumConst() { isInsecureEncryption(result.ge EnumConstant getAdditionalEvidenceEnumConst() { isEncryptionAdditionalEvidence(result.getName()) } /** - * A function call we have a high confidence is related to use of an insecure - * encryption algorithm. + * A function call we have a high confidence is related to use of an insecure encryption algorithm, along + * with an associated `Element` which might be the best point to blame, and a description of that element. */ -class InsecureFunctionCall extends FunctionCall { - Element blame; - string explain; +predicate getInsecureEncryptionEvidence(FunctionCall fc, Element blame, string description) { + // find use of an insecure algorithm name + ( + fc.getTarget() = getAnInsecureEncryptionFunction() and + blame = fc and + description = "call to " + fc.getTarget().getName() + or + exists(MacroInvocation mi | + ( + mi.getAnExpandedElement() = fc or + mi.getAnExpandedElement() = fc.getAnArgument() + ) and + mi.getMacro() = getAnInsecureEncryptionMacro() and + blame = mi and + description = "invocation of macro " + mi.getMacro().getName() + ) + or + exists(EnumConstantAccess ec | + ec = fc.getAnArgument() and + ec.getTarget() = getAnInsecureEncryptionEnumConst() and + blame = ec and + description = "access of enum constant " + ec.getTarget().getName() + ) + ) and + // find additional evidence that this function is related to encryption. + ( + fc.getTarget() = getAnAdditionalEvidenceFunction() + or + exists(MacroInvocation mi | + ( + mi.getAnExpandedElement() = fc or + mi.getAnExpandedElement() = fc.getAnArgument() + ) and + mi.getMacro() = getAnAdditionalEvidenceMacro() + ) + or + exists(EnumConstantAccess ec | + ec = fc.getAnArgument() and + ec.getTarget() = getAdditionalEvidenceEnumConst() + ) + ) +} + +/** + * An element that is the `blame` of an `InsecureFunctionCall`. + */ +class BlamedElement extends Element { + string description; + + BlamedElement() { getInsecureEncryptionEvidence(_, this, description) } - 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() - ) + /** + * Holds if this is the `num`-th `BlamedElement` in `f`. + */ + predicate hasFileRank(File f, int num) { + exists(int loc | + getLocation().charLoc(f, loc, _) and + loc = + rank[num](BlamedElement other, int loc2 | other.getLocation().charLoc(f, loc2, _) | loc2) ) } - Element getBlame() { result = blame } - - string getDescription() { result = explain } + string getDescription() { result = description } } -from InsecureFunctionCall c -select c.getBlame(), - "This " + c.getDescription() + " specifies a broken or weak cryptographic algorithm." +from File f, BlamedElement firstResult, BlamedElement thisResult +where + firstResult.hasFileRank(f, 1) and + thisResult.hasFileRank(f, _) +select firstResult, + "This file makes use of a broken or weak cryptographic algorithm (specified by $@).", thisResult, + thisResult.getDescription() diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-327/BrokenCryptoAlgorithm.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-327/BrokenCryptoAlgorithm.expected index 19cdfa5e9c73..d27d5c4bbca7 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-327/BrokenCryptoAlgorithm.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-327/BrokenCryptoAlgorithm.expected @@ -1,25 +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. | +| 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:49:4:49:24 | call to my_des_implementation | call to my_des_implementation | +| 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:62:33:62:40 | ALGO_DES | invocation of macro ALGO_DES | +| 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:124:4:124:24 | call to my_des_implementation | call to my_des_implementation | +| 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:144:27:144:29 | DES | access of enum constant DES | +| 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:172:28:172:35 | ALGO_DES | invocation of macro ALGO_DES | +| 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:175:28:175:34 | USE_DES | access of enum constant USE_DES | +| 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:182:38:182:45 | ALGO_DES | invocation of macro ALGO_DES | +| 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:185:38:185:44 | USE_DES | access of enum constant USE_DES | +| 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:238:2:238:20 | call to encrypt | call to encrypt | +| 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:245:5:245:11 | call to encrypt | call to encrypt | +| 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 | +| 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 | +| 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 | +| 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:42:2:42:38 | ENCRYPT_WITH_TRIPLE_DES(data,amount) | invocation of macro ENCRYPT_WITH_TRIPLE_DES | +| 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:51:2:51:32 | DES_DO_ENCRYPTION(data,amount) | invocation of macro DES_DO_ENCRYPTION | +| 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:52:2:52:31 | RUN_DES_ENCODING(data,amount) | invocation of macro RUN_DES_ENCODING | +| 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:53:2:53:25 | DES_ENCODE(data,amount) | invocation of macro DES_ENCODE | +| 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:54:2:54:26 | DES_SET_KEY(data,amount) | invocation of macro DES_SET_KEY | +| 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:88:2:88:11 | call to encryptDES | call to encryptDES | +| 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:89:2:89:11 | call to encryptRC2 | call to encryptRC2 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 |