Skip to content

Commit 9f09e67

Browse files
bdrodesbdrodes
committed
Adding BannedEncryption qhelp, ql, cap and cng qll, cryptofilters qll and misc. crypto example files
1 parent 9668fb3 commit 9f09e67

File tree

13 files changed

+485
-0
lines changed

13 files changed

+485
-0
lines changed

‎cpp/ql/src/Microsoft/Security/Cryptography/BannedEncryption.qhelp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Finds explicit uses of symmetric encryption algorithms that are weak, obsolete, or otherwise unapproved.
9+
</p>
10+
<p>
11+
Encryption algorithms such as DES, (uses keys of 56 bits only), RC2 (uses keys of 128 bits only), and TripleDES (provides at most 112 bits of security) are considered to be weak.
12+
</p>
13+
<p>
14+
These cryptographic algorithms do not provide as much security assurance as more modern counterparts.
15+
</p>
16+
</overview>
17+
18+
<recommendation>
19+
<p>
20+
For Microsoft internal security standards:
21+
</p>
22+
<p>
23+
For WinCrypt, switch to ALG_SID_AES, ALG_SID_AES_128, ALG_SID_AES_192, or ALG_SID_AES_256.
24+
</p>
25+
<p>
26+
For BCrypt, switch to AES or any algorithm other than RC2, RC4, DES, DESX, 3DES, 3DES_112. AES_GMAC and AES_CMAC require crypto board review.
27+
</p>
28+
</recommendation>
29+
30+
<example>
31+
<p>Violations:</p>
32+
33+
<sample src="examples/WeakEncryption/WeakEncryption1.cpp" />
34+
<sample src="examples/WeakEncryption/WeakEncryption3.cpp" />
35+
36+
<p>Solutions:</p>
37+
38+
<sample src="examples/WeakEncryption/WeakEncryption2.cpp" />
39+
<sample src="examples/WeakEncryption/WeakEncryption4.cpp" />
40+
</example>
41+
42+
<references>
43+
<li>Microsoft Docs: <a href="https://learn.microsoft.com/en-us/security/engineering/cryptographic-recommendations">Microsoft SDL Cryptographic Recommendations</a>.</li>
44+
</references>
45+
46+
</qhelp>

‎cpp/ql/src/Microsoft/Security/Cryptography/BannedEncryption.ql

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/**
2+
* @name Weak cryptography
3+
* @description Finds explicit uses of symmetric encryption algorithms that are weak, obsolete, or otherwise unapproved.
4+
* @kind problem
5+
* @id cpp/weak-crypto/banned-encryption-algorithms
6+
* @problem.severity error
7+
* @precision high
8+
* @tags security
9+
* external/cwe/cwe-327
10+
*/
11+
12+
import cpp
13+
import CryptoFilters
14+
import CryptoDataflowCapi
15+
import CryptoDataflowCng
16+
import experimental.cryptography.Concepts
17+
18+
predicate isCapiOrCNGBannedAlg(Expr e, string msg) {
19+
exists(FunctionCall fc |
20+
CapiCryptCreateEncryptionBanned::flow(DataFlow::exprNode(e),
21+
DataFlow::exprNode(fc.getArgument(1)))
22+
or
23+
BCryptOpenAlgorithmProviderBannedEncryption::flow(DataFlow::exprNode(e),
24+
DataFlow::exprNode(fc.getArgument(1)))
25+
) and
26+
msg =
27+
"Call to a cryptographic function with a banned symmetric encryption algorithm: " +
28+
e.getValueText()
29+
}
30+
31+
predicate isGeneralBannedAlg(SymmetricEncryptionAlgorithm alg, Expr confSink, string msg) {
32+
// Handle unknown cases in a separate query
33+
not alg.getEncryptionName() = unknownAlgorithm() and
34+
exists(string resMsg |
35+
(
36+
not alg.getEncryptionName().matches("AES%") and
37+
resMsg = "Use of banned symmetric encryption algorithm: " + alg.getEncryptionName() + "."
38+
) and
39+
(
40+
if alg.hasConfigurationSink() and alg.configurationSink() != alg
41+
then (
42+
confSink = alg.configurationSink() and msg = resMsg + " Algorithm used at sink: $@."
43+
) else (
44+
confSink = alg and msg = resMsg
45+
)
46+
)
47+
)
48+
}
49+
50+
from Expr sink, Expr confSink, string msg
51+
where
52+
(
53+
isCapiOrCNGBannedAlg(sink, msg) and confSink = sink
54+
or
55+
isGeneralBannedAlg(sink, confSink, msg)
56+
) and
57+
not isSrcSinkFiltered(sink, confSink)
58+
select sink, msg, confSink, confSink.toString()

‎cpp/ql/src/Microsoft/Security/Cryptography/CryptoDataflowCapi.qll

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/**
2+
* Provides classes and predicates for identifying expressions that are use Crypto API (CAPI).
3+
*/
4+
5+
import cpp
6+
private import semmle.code.cpp.dataflow.new.DataFlow
7+
8+
/**
9+
* Dataflow that detects a call to CryptSetKeyParam dwParam = KP_MODE (CAPI)
10+
*/
11+
module CapiCryptCryptSetKeyParamtoKPMODEConfiguration implements DataFlow::ConfigSig {
12+
predicate isSource(DataFlow::Node source) {
13+
source.asExpr().getValue().toInt() = 4 // KP_MODE
14+
}
15+
16+
predicate isSink(DataFlow::Node sink) {
17+
exists(FunctionCall call |
18+
// CryptSetKeyParam 2nd argument specifies the key parameter to set
19+
sink.asExpr() = call.getArgument(1) and
20+
call.getTarget().hasGlobalName("CryptSetKeyParam")
21+
)
22+
}
23+
}
24+
25+
module CapiCryptCryptSetKeyParamtoKPMODE =
26+
DataFlow::Global<CapiCryptCryptSetKeyParamtoKPMODEConfiguration>;
27+
28+
/**
29+
* A function call to CryptSetKeyParam with dwParam = KP_MODE (CAPI)
30+
*/
31+
class CapiCryptCryptSetKeyParamtoKPMODE extends FunctionCall {
32+
CapiCryptCryptSetKeyParamtoKPMODE() {
33+
exists(Expr var |
34+
CapiCryptCryptSetKeyParamtoKPMODE::flow(DataFlow::exprNode(var),
35+
DataFlow::exprNode(this.getArgument(1)))
36+
)
37+
}
38+
}
39+
40+
// CAPI-specific DataFlow configuration
41+
module CapiCryptCreateHashBannedConfiguration implements DataFlow::ConfigSig {
42+
// This mechnism will verify for approved set of values to call, rejecting anythign that is not in the list.
43+
// NOTE: This mechanism is not guaranteed to work with CSPs that do not use the same algorithms defined in Wincrypt.h
44+
//
45+
predicate isSource(DataFlow::Node source) {
46+
// Verify if source matched the mask for CAPI ALG_CLASS_HASH == 32768
47+
source.asExpr().getValue().toInt().bitShiftRight(13) = 4 and
48+
// The following hash algorithms are safe to use, anything else is considered banned
49+
not (
50+
source.asExpr().getValue().toInt().bitXor(32768) = 12 or // ALG_SID_SHA_256
51+
source.asExpr().getValue().toInt().bitXor(32768) = 13 or // ALG_SID_SHA_384
52+
source.asExpr().getValue().toInt().bitXor(32768) = 14 // ALG_SID_SHA_512
53+
)
54+
}
55+
56+
predicate isSink(DataFlow::Node sink) {
57+
exists(FunctionCall call |
58+
// CryptCreateHash 2nd argument specifies the hash algorithm to be used.
59+
sink.asExpr() = call.getArgument(1) and
60+
call.getTarget().hasGlobalName("CryptCreateHash")
61+
)
62+
}
63+
}
64+
65+
module CapiCryptCreateHashBanned = DataFlow::Global<CapiCryptCreateHashBannedConfiguration>;
66+
67+
// CAPI-specific DataFlow configuration
68+
module CapiCryptCreateEncryptionBannedConfiguration implements DataFlow::ConfigSig {
69+
// This mechanism will verify for approved set of values to call, rejecting anything that is not in the list.
70+
// NOTE: This mechanism is not guaranteed to work with CSPs that do not use the same algorithms defined in Wincrypt.h
71+
//
72+
predicate isSource(DataFlow::Node source) {
73+
// Verify if source matched the mask for CAPI ALG_CLASS_DATA_ENCRYPT == 24576
74+
source.asExpr().getValue().toInt().bitShiftRight(13) = 3 and
75+
// The following algorithms are safe to use, anything else is considered banned
76+
not (
77+
source.asExpr().getValue().toInt().bitXor(26112) = 14 or // ALG_SID_AES_128
78+
source.asExpr().getValue().toInt().bitXor(26112) = 15 or // ALG_SID_AES_192
79+
source.asExpr().getValue().toInt().bitXor(26112) = 16 or // ALG_SID_AES_256
80+
source.asExpr().getValue().toInt().bitXor(26112) = 17 // ALG_SID_AES
81+
)
82+
}
83+
84+
predicate isSink(DataFlow::Node sink) {
85+
exists(FunctionCall call |
86+
// CryptGenKey or CryptDeriveKey 2nd argument specifies the hash algorithm to be used.
87+
sink.asExpr() = call.getArgument(1) and
88+
(
89+
call.getTarget().hasGlobalName("CryptGenKey") or
90+
call.getTarget().hasGlobalName("CryptDeriveKey")
91+
)
92+
)
93+
}
94+
}
95+
96+
module CapiCryptCreateEncryptionBanned =
97+
DataFlow::Global<CapiCryptCreateEncryptionBannedConfiguration>;

‎cpp/ql/src/Microsoft/Security/Cryptography/CryptoDataflowCng.qll

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/**
2+
* Provides classes and predicates for identifying expressions that are use crypto API Next Generation (CNG).
3+
*/
4+
5+
import cpp
6+
private import semmle.code.cpp.dataflow.new.DataFlow
7+
8+
/**
9+
* Dataflow that detects a call to BCryptSetProperty pszProperty = ChainingMode (CNG)
10+
*/
11+
module CngBCryptSetPropertyParamtoKChainingModeConfiguration implements DataFlow::ConfigSig {
12+
predicate isSource(DataFlow::Node source) {
13+
source.asExpr().getValue().toString().matches("ChainingMode")
14+
}
15+
16+
predicate isSink(DataFlow::Node sink) {
17+
exists(FunctionCall call |
18+
// BCryptSetProperty 2nd argument specifies the key parameter to set
19+
sink.asExpr() = call.getArgument(1) and
20+
call.getTarget().hasGlobalName("BCryptSetProperty")
21+
)
22+
}
23+
}
24+
25+
module CngBCryptSetPropertyParamtoKChainingMode =
26+
DataFlow::Global<CngBCryptSetPropertyParamtoKChainingModeConfiguration>;
27+
28+
/**
29+
* A function call to BCryptSetProperty pszProperty = ChainingMode (CNG)
30+
*/
31+
class CngBCryptSetPropertyParamtoKChainingMode extends FunctionCall {
32+
CngBCryptSetPropertyParamtoKChainingMode() {
33+
exists(Expr var |
34+
CngBCryptSetPropertyParamtoKChainingMode::flow(DataFlow::exprNode(var),
35+
DataFlow::exprNode(this.getArgument(1)))
36+
)
37+
}
38+
}
39+
40+
predicate isChaniningModeCbc(DataFlow::Node source) {
41+
// Verify if algorithm is in the approved list.
42+
exists(string s | s = source.asExpr().getValue().toString() |
43+
s.regexpMatch("ChainingMode[A-Za-z0-9/]+") and
44+
// Property Strings
45+
// BCRYPT_CHAIN_MODE_NA L"ChainingModeN/A" - The algorithm does not support chaining
46+
// BCRYPT_CHAIN_MODE_CBC L"ChainingModeCBC" - Microsoft-Only: Only mode allowed by Crypto Board from this list (CBC-MAC)
47+
// BCRYPT_CHAIN_MODE_ECB L"ChainingModeECB" - Generally not recommended for usage in cryptographic protocols at all
48+
// BCRYPT_CHAIN_MODE_CFB L"ChainingModeCFB" - Microsoft-Only: Banned, usage requires Crypto Board review
49+
// BCRYPT_CHAIN_MODE_CCM L"ChainingModeCCM" - Microsoft-Only: Banned, usage requires Crypto Board review
50+
// BCRYPT_CHAIN_MODE_GCM L"ChainingModeGCM" - Microsoft-Only: Only for TLS, other usage requires Crypto Board review
51+
not s.matches("ChainingModeCBC")
52+
)
53+
}
54+
55+
module CngBCryptSetPropertyChainingBannedModeConfiguration implements DataFlow::ConfigSig {
56+
predicate isSource(DataFlow::Node source) { isChaniningModeCbc(source) }
57+
58+
predicate isSink(DataFlow::Node sink) {
59+
exists(CngBCryptSetPropertyParamtoKChainingMode call |
60+
// BCryptOpenAlgorithmProvider 3rd argument sets the chaining mode value
61+
sink.asExpr() = call.getArgument(2)
62+
)
63+
}
64+
}
65+
66+
module CngBCryptSetPropertyChainingBannedMode =
67+
DataFlow::Global<CngBCryptSetPropertyChainingBannedModeConfiguration>;
68+
69+
module CngBCryptSetPropertyChainingBannedModeIndirectParameterConfiguration implements
70+
DataFlow::ConfigSig
71+
{
72+
predicate isSource(DataFlow::Node source) { isChaniningModeCbc(source) }
73+
74+
predicate isSink(DataFlow::Node sink) {
75+
exists(CngBCryptSetPropertyParamtoKChainingMode call |
76+
// CryptSetKeyParam 3rd argument specifies the mode (KP_MODE)
77+
sink.asIndirectExpr() = call.getArgument(2)
78+
)
79+
}
80+
}
81+
82+
module CngBCryptSetPropertyChainingBannedModeIndirectParameter =
83+
DataFlow::Global<CngBCryptSetPropertyChainingBannedModeIndirectParameterConfiguration>;
84+
85+
// CNG-specific DataFlow configuration
86+
module BCryptOpenAlgorithmProviderBannedHashConfiguration implements DataFlow::ConfigSig {
87+
// NOTE: Unlike the CAPI scenario, CNG will use this method to load and initialize
88+
// a cryptographic provider for any type of algorithm,not only hash.
89+
// Therefore, we have to take a banned-list instead of approved list approach.
90+
//
91+
predicate isSource(DataFlow::Node source) {
92+
// Verify if algorithm is marked as banned.
93+
source.asExpr().getValue().toString().matches("MD_")
94+
or
95+
source.asExpr().getValue().toString().matches("SHA1")
96+
}
97+
98+
predicate isSink(DataFlow::Node sink) {
99+
exists(FunctionCall call |
100+
// BCryptOpenAlgorithmProvider 2nd argument specifies the algorithm to be used
101+
sink.asExpr() = call.getArgument(1) and
102+
call.getTarget().hasGlobalName("BCryptOpenAlgorithmProvider")
103+
)
104+
}
105+
}
106+
107+
module BCryptOpenAlgorithmProviderBannedHash =
108+
DataFlow::Global<BCryptOpenAlgorithmProviderBannedHashConfiguration>;
109+
110+
// CNG-specific DataFlow configuration
111+
module BCryptOpenAlgorithmProviderBannedEncryptionConfiguration implements DataFlow::ConfigSig {
112+
// NOTE: Unlike the CAPI scenario, CNG will use this method to load and initialize
113+
// a cryptographic provider for any type of algorithm,not only encryption.
114+
// Therefore, we have to take a banned-list instead of approved list approach.
115+
//
116+
predicate isSource(DataFlow::Node source) {
117+
// Verify if algorithm is marked as banned.
118+
source.asExpr().getValue().toString().matches("RC_") or
119+
source.asExpr().getValue().toString().matches("DES") or
120+
source.asExpr().getValue().toString().matches("DESX") or
121+
source.asExpr().getValue().toString().matches("3DES") or
122+
source.asExpr().getValue().toString().matches("3DES_112") or
123+
source.asExpr().getValue().toString().matches("AES_GMAC") or // Microsoft Only: Requires Cryptoboard review
124+
source.asExpr().getValue().toString().matches("AES_CMAC") // Microsoft Only: Requires Cryptoboard review
125+
}
126+
127+
predicate isSink(DataFlow::Node sink) {
128+
exists(FunctionCall call |
129+
// BCryptOpenAlgorithmProvider 2nd argument specifies the algorithm to be used
130+
sink.asExpr() = call.getArgument(1) and
131+
call.getTarget().hasGlobalName("BCryptOpenAlgorithmProvider")
132+
)
133+
}
134+
}
135+
136+
module BCryptOpenAlgorithmProviderBannedEncryption =
137+
DataFlow::Global<BCryptOpenAlgorithmProviderBannedEncryptionConfiguration>;

‎cpp/ql/src/Microsoft/Security/Cryptography/CryptoFilters.qll

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import cpp
2+
3+
/**
4+
* Determines if an element should be filtered (ignored)
5+
* from any result set.
6+
*
7+
* The current strategy is to determine if the element
8+
* resides in a path that appears to be a library (in particular openssl).
9+
*
10+
* It is therefore important that the element being examined represents
11+
* a use or configuration of cryptography in the user code.
12+
* E.g., if a global variable were defined in an OpenSSL library
13+
* representing a bad/vuln algorithm, and this global were assessed
14+
* it would appear to be ignorable, as it exists in a a filtered library.
15+
* The use of that global must be examined with this filter.
16+
*
17+
* ASSUMPTION/CAVEAT: note if an openssl library wraps a dangerous crypo use
18+
* this filter approach will ignore the wrapper call, unless it is also flagged
19+
* as dangerous. e.g., SomeWraper(){ ... <md5 use> ...}
20+
* The wrapper if defined in openssl would result in ignoring
21+
* the use of MD5 internally, since it's use is entirely in openssl.
22+
*
23+
* TODO: these caveats need to be reassessed in the future.
24+
*/
25+
predicate isUseFiltered(Element e) {
26+
e.getFile().getAbsolutePath().toLowerCase().matches("%openssl%")
27+
}
28+
29+
/**
30+
* Filtered only if both src and sink are considered filtered.
31+
*
32+
* This approach is meant to partially address some of the implications of
33+
* `isUseFiltered`. Specifically, if an algorithm is specified by a user
34+
* and some how passes to a user inside openssl, then this filter
35+
* would not ignore that the user was specifying the use of something dangerous.
36+
*
37+
* e.g., if a wrapper in openssl existed of the form SomeWrapper(string alg, ...){ ... <operation using alg> ...}
38+
* and the user did something like SomeWrapper("MD5", ...), this would not be ignored.
39+
*
40+
* The source in the above example would the algorithm, and the sink is the configuration sink
41+
* of the algorithm.
42+
*/
43+
predicate isSrcSinkFiltered(Element src, Element sink) {
44+
isUseFiltered(src) and isUseFiltered(sink)
45+
}

0 commit comments

Comments
 (0)