Skip to content

Commit 34fe60d

Browse files
bdrodesbdrodes
committed
KDF ql and qhelp
1 parent 5d3f35b commit 34fe60d

8 files changed

+285
-0
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Use of KDF algorithm BCryptDeriveKeyPBKDF2 uses insecure hash from BCryptOpenAlgorithmProvider.</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>Use SHA 256, 384, or 512.</p>
12+
</recommendation>
13+
14+
</qhelp>

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

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* @name KDF may only use SHA256/384/512 in generating a key.
3+
* @description KDF may only use SHA256/384/512 in generating a key.
4+
* @kind problem
5+
* @id cpp/kdf-insecure-hash
6+
* @problem.severity error
7+
* @precision high
8+
* @tags security
9+
*/
10+
11+
import cpp
12+
import semmle.code.cpp.dataflow.new.DataFlow
13+
14+
module BannedHashAlgorithmConfig implements DataFlow::ConfigSig {
15+
predicate isSource(DataFlow::Node source) {
16+
// Verify if algorithm is marked as banned.
17+
not source.asExpr().getValue().toString().matches("SHA256") and
18+
not source.asExpr().getValue().toString().matches("SHA384") and
19+
not source.asExpr().getValue().toString().matches("SHA512")
20+
}
21+
22+
predicate isSink(DataFlow::Node sink) {
23+
exists(FunctionCall call |
24+
// Argument 1 (0-based) specified the algorithm ID.
25+
// NTSTATUS BCryptOpenAlgorithmProvider(
26+
// [out] BCRYPT_ALG_HANDLE *phAlgorithm,
27+
// [in] LPCWSTR pszAlgId,
28+
// [in] LPCWSTR pszImplementation,
29+
// [in] ULONG dwFlags
30+
// );
31+
sink.asExpr() = call.getArgument(1) and
32+
call.getTarget().hasGlobalName("BCryptOpenAlgorithmProvider")
33+
)
34+
}
35+
}
36+
37+
module BannedHashAlgorithmTrace = DataFlow::Global<BannedHashAlgorithmConfig>;
38+
39+
module BCRYPT_ALG_HANDLE_Config implements DataFlow::ConfigSig {
40+
predicate isSource(DataFlow::Node source) {
41+
exists(FunctionCall call |
42+
// Argument 0 (0-based) specified the algorithm handle
43+
// NTSTATUS BCryptOpenAlgorithmProvider(
44+
// [out] BCRYPT_ALG_HANDLE *phAlgorithm,
45+
// [in] LPCWSTR pszAlgId,
46+
// [in] LPCWSTR pszImplementation,
47+
// [in] ULONG dwFlags
48+
// );
49+
source.asDefiningArgument() = call.getArgument(0) and
50+
call.getTarget().hasGlobalName("BCryptOpenAlgorithmProvider")
51+
)
52+
}
53+
54+
predicate isSink(DataFlow::Node sink) {
55+
// Algorithm handle is the 0th (0-based) argument of the call
56+
// NTSTATUS BCryptDeriveKeyPBKDF2(
57+
// [in] BCRYPT_ALG_HANDLE hPrf,
58+
// [in, optional] PUCHAR pbPassword,
59+
// [in] ULONG cbPassword,
60+
// [in, optional] PUCHAR pbSalt,
61+
// [in] ULONG cbSalt,
62+
// [in] ULONGLONG cIterations,
63+
// [out] PUCHAR pbDerivedKey,
64+
// [in] ULONG cbDerivedKey,
65+
// [in] ULONG dwFlags
66+
// );
67+
exists(Call c | c.getTarget().getName() = "BCryptDeriveKeyPBKDF2" |
68+
c.getArgument(0) = sink.asExpr()
69+
)
70+
}
71+
}
72+
73+
module BCRYPT_ALG_HANDLE_Trace = DataFlow::Global<BCRYPT_ALG_HANDLE_Config>;
74+
75+
from DataFlow::Node src1, DataFlow::Node src2, DataFlow::Node sink1, DataFlow::Node sink2
76+
where
77+
BannedHashAlgorithmTrace::flow(src1, sink1) and
78+
exists(Call c |
79+
c.getAnArgument() = sink1.asExpr() and src2.asDefiningArgument() = c.getAnArgument()
80+
|
81+
BCRYPT_ALG_HANDLE_Trace::flow(src2, sink2)
82+
)
83+
select sink2.asExpr(),
84+
"BCRYPT_ALG_HANDLE is passed to this to KDF derived from insecure hashing function $@. Must use SHA256 or higher.",
85+
src1.asExpr(), src1.asExpr().getValue()

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Use of KDF algorithm BCryptDeriveKeyPBKDF2 uses low iteration count (less than 100k).</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>Use a minimum of 100,000 for iteration count.</p>
12+
</recommendation>
13+
14+
</qhelp>

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/**
2+
* @name Use iteration count at least 100k to prevent brute force attacks
3+
* @description When deriving cryptographic keys from user-provided inputs such as password, use sufficient iteration count (at least 100k).
4+
* This query traces constants of <100k to the iteration count parameter of CNG's BCryptDeriveKeyPBKDF2.
5+
* This query traces constants of less than the min length to the target parameter.
6+
* NOTE: if the constant is modified, or if a non-constant gets to the iteration count, this query will not flag these cases.
7+
* The rationale currently is that this query is meant to validate common uses of key derivation.
8+
* Non-common uses (modifying the iteration count somehow or getting the count from outside sources) are assumed to be intentional.
9+
* @kind problem
10+
* @id cpp/kdf-low-iteration-count
11+
* @problem.severity error
12+
* @precision high
13+
* @tags security
14+
*/
15+
16+
import cpp
17+
import semmle.code.cpp.dataflow.new.DataFlow
18+
19+
module IterationCountDataFlowConfig implements DataFlow::ConfigSig {
20+
/**
21+
* Defines the source for iteration count when it's coming from a fixed value
22+
* Any expression that has an assigned value < 100000 could be a source.
23+
*/
24+
predicate isSource(DataFlow::Node src) { src.asExpr().getValue().toInt() < 100000 }
25+
26+
predicate isSink(DataFlow::Node sink) {
27+
// iterations count is the 5th (0-based) argument of the call
28+
// NTSTATUS BCryptDeriveKeyPBKDF2(
29+
// [in] BCRYPT_ALG_HANDLE hPrf,
30+
// [in, optional] PUCHAR pbPassword,
31+
// [in] ULONG cbPassword,
32+
// [in, optional] PUCHAR pbSalt,
33+
// [in] ULONG cbSalt,
34+
// [in] ULONGLONG cIterations,
35+
// [out] PUCHAR pbDerivedKey,
36+
// [in] ULONG cbDerivedKey,
37+
// [in] ULONG dwFlags
38+
// );
39+
exists(Call c | c.getTarget().getName() = "BCryptDeriveKeyPBKDF2" |
40+
c.getArgument(5) = sink.asExpr()
41+
)
42+
}
43+
}
44+
45+
module IterationCountDataFlow = DataFlow::Global<IterationCountDataFlowConfig>;
46+
47+
from DataFlow::Node src, DataFlow::Node sink
48+
where IterationCountDataFlow::flow(src, sink)
49+
select sink.asExpr(),
50+
"Iteration count $@ is passed to this to KDF. Use at least 100000 iterations when deriving cryptographic key from password.",
51+
src, src.asExpr().getValue()

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Use of KDF algorithm BCryptDeriveKeyPBKDF2 uses small key size (less than 16 bytes).</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>Use a minimum of 16 bytes for key size.</p>
12+
</recommendation>
13+
14+
</qhelp>

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/**
2+
* @name Small KDF derived key length.
3+
* @description KDF derived keys should be a minimum of 128 bits (16 bytes).
4+
* This query traces constants of less than the min length to the target parameter.
5+
* NOTE: if the constant is modified, or if a non-constant gets to the target, this query will not flag these cases.
6+
* The rationale currently is that this query is meant to validate common uses of key derivation.
7+
* Non-common uses (modifying the values somehow or getting the count from outside sources) are assumed to be intentional.
8+
* @kind problem
9+
* @id cpp/kdf-small-key-size
10+
* @problem.severity error
11+
* @precision high
12+
* @tags security
13+
*/
14+
15+
import cpp
16+
import semmle.code.cpp.dataflow.new.DataFlow
17+
18+
module KeyLenConfig implements DataFlow::ConfigSig {
19+
predicate isSource(DataFlow::Node src) { src.asExpr().getValue().toInt() < 16 }
20+
21+
predicate isSink(DataFlow::Node sink) {
22+
// Key length size is the 7th (0-based) argument of the call
23+
// NTSTATUS BCryptDeriveKeyPBKDF2(
24+
// [in] BCRYPT_ALG_HANDLE hPrf,
25+
// [in, optional] PUCHAR pbPassword,
26+
// [in] ULONG cbPassword,
27+
// [in, optional] PUCHAR pbSalt,
28+
// [in] ULONG cbSalt,
29+
// [in] ULONGLONG cIterations,
30+
// [out] PUCHAR pbDerivedKey,
31+
// [in] ULONG cbDerivedKey,
32+
// [in] ULONG dwFlags
33+
// );
34+
exists(Call c | c.getTarget().getName() = "BCryptDeriveKeyPBKDF2" |
35+
c.getArgument(7) = sink.asExpr()
36+
)
37+
}
38+
}
39+
40+
module KeyLenTrace = DataFlow::Global<KeyLenConfig>;
41+
42+
from DataFlow::Node src, DataFlow::Node sink
43+
where KeyLenTrace::flow(src, sink)
44+
select sink.asExpr(),
45+
"Key size $@ is passed to this to KDF. Use at least 16 bytes for key length when deriving cryptographic key from password.",
46+
src.asExpr(), src.asExpr().getValue()

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Use of KDF algorithm BCryptDeriveKeyPBKDF2 uses small salt size (less than 16 bytes).</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>Use a minimum of 16 bytes for salt size.</p>
12+
</recommendation>
13+
14+
15+
</qhelp>

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/**
2+
* @name Small KDF salt length.
3+
* @description KDF salts should be a minimum of 128 bits (16 bytes).
4+
* This query traces constants of less than the min length to the target parameter.
5+
* NOTE: if the constant is modified, or if a non-constant gets to the target, this query will not flag these cases.
6+
* The rationale currently is that this query is meant to validate common uses of key derivation.
7+
* Non-common uses (modifying the values somehow or getting the count from outside sources) are assumed to be intentional.
8+
* @kind problem
9+
* @id cpp/kdf-small-salt-size
10+
* @problem.severity error
11+
* @precision high
12+
* @tags security
13+
*/
14+
15+
import cpp
16+
import semmle.code.cpp.dataflow.new.DataFlow
17+
18+
module SaltLenConfig implements DataFlow::ConfigSig {
19+
predicate isSource(DataFlow::Node src) { src.asExpr().getValue().toInt() < 16 }
20+
21+
predicate isSink(DataFlow::Node sink) {
22+
// Key length size is the 7th (0-based) argument of the call
23+
// NTSTATUS BCryptDeriveKeyPBKDF2(
24+
// [in] BCRYPT_ALG_HANDLE hPrf,
25+
// [in, optional] PUCHAR pbPassword,
26+
// [in] ULONG cbPassword,
27+
// [in, optional] PUCHAR pbSalt,
28+
// [in] ULONG cbSalt,
29+
// [in] ULONGLONG cIterations,
30+
// [out] PUCHAR pbDerivedKey,
31+
// [in] ULONG cbDerivedKey,
32+
// [in] ULONG dwFlags
33+
// );
34+
exists(Call c | c.getTarget().getName() = "BCryptDeriveKeyPBKDF2" |
35+
c.getArgument(4) = sink.asExpr()
36+
)
37+
}
38+
}
39+
40+
module SaltLenTrace = DataFlow::Global<SaltLenConfig>;
41+
42+
from DataFlow::Node src, DataFlow::Node sink
43+
where SaltLenTrace::flow(src, sink)
44+
select sink.asExpr(),
45+
"Salt size $@ is passed to this to KDF. Use at least 16 bytes for salt size when deriving cryptographic key from password.",
46+
src, src.asExpr().getValue()

0 commit comments

Comments
 (0)