Skip to content

Commit 1376b0c

Browse files
bdrodesbdrodes
committed
Deprecated and hardcoded protocol queries and help
1 parent 69cbbff commit 1376b0c

File tree

9 files changed

+313
-0
lines changed

9 files changed

+313
-0
lines changed

‎cpp/ql/src/Microsoft/Security/Protocols/HardCodedSecurityProtocol.qhelp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Hard-coding security protocols rather than specifying the system default is risky because the protocol may become deprecated in future.</p>
8+
<p>The <code>grbitEnabledProtocols</code> member of the <code>SCHANNEL_CRED</code> struct contains a bit string that represents the protocols supported by connections made with credentials acquired by using this structure. If this member is zero, Schannel selects the protocol. Applications should set <code>grbitEnabledProtocols</code> to zero and use the protocol versions enabled on the system by default.</p>
9+
</overview>
10+
11+
<recommendation>
12+
<p> - Set the <code>grbitEnabledProtocols</code> member of the <code>SCHANNEL_CRED</code> struct to <code>0</code>.</p>
13+
</recommendation>
14+
15+
<example>
16+
<p>Violation:</p>
17+
18+
<sample src="examples/HardCodedSecurityProtocol/HardCodedSecurityProtocol1.cpp" />
19+
20+
<p>Solution:</p>
21+
22+
<sample src="examples/HardCodedSecurityProtocol/HardCodedSecurityProtocol2.cpp" />
23+
</example>
24+
25+
<references>
26+
<li>Microsoft Docs: <a href="https://learn.microsoft.com/en-us/windows/win32/api/schannel/ns-schannel-schannel_cred">SCHANNEL_CRED structure</a>.</li>
27+
</references>
28+
29+
</qhelp>

‎cpp/ql/src/Microsoft/Security/Protocols/HardCodedSecurityProtocol.ql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @name Hard-coded use of a security protocol
3+
* @description Hard-coding the security protocol used rather than specifying the system default is
4+
* risky because the protocol may become deprecated in future.
5+
* @kind problem
6+
* @problem.severity warning
7+
* @id cpp/hardcoded-security-protocol
8+
*/
9+
10+
import cpp
11+
import HardCodedSecurityProtocol
12+
13+
from ProtocolConstant constantValue, DataFlow::Node grbitEnabledProtocolsAssignment
14+
where
15+
GrbitEnabledConstantTace::flow(DataFlow::exprNode(constantValue), grbitEnabledProtocolsAssignment) and
16+
constantValue.isHardcodedProtocol()
17+
select constantValue,
18+
"Hard-coded use of security protocol " + getConstantName(constantValue) + " set here $@.",
19+
grbitEnabledProtocolsAssignment, grbitEnabledProtocolsAssignment.toString()

‎cpp/ql/src/Microsoft/Security/Protocols/HardCodedSecurityProtocol.qll

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
import cpp
2+
import semmle.code.cpp.dataflow.new.TaintTracking
3+
4+
/**
5+
* A constant representing one or more security protocols for the `grbitEnabledProtocols` field.
6+
*/
7+
class ProtocolConstant extends Expr {
8+
ProtocolConstant() {
9+
this.isConstant() and
10+
GrbitEnabledConstantTace::flow(DataFlow::exprNode(this), _) and
11+
(
12+
this instanceof Literal
13+
or
14+
this = any(ConstantMacroInvocation mi).getExpr()
15+
or
16+
// This is a workaround for folded constants, which currently have no
17+
// dataflow node representation. Attach to the outermost dataflow node
18+
// where a literal exists as a child that has no dataflow node representation.
19+
exists(Literal l |
20+
this.getAChild*() = l and
21+
not exists(DataFlow::Node n | n.asExpr() = l)
22+
)
23+
)
24+
}
25+
26+
/** Gets the bitmask represented by this constant. */
27+
int getBitmask() { result = this.getValue().toInt() }
28+
29+
/** Holds if this constant only represents TLS1.3 protocols. */
30+
predicate isTLS1_3Only() {
31+
// Flags for TLS1.3 are 0x00001000 and 0x00002000
32+
// 12288 = 0x00001000 | 0x00002000
33+
this.getBitmask().bitAnd(12288.bitNot()) = 0 and
34+
not this.isSystemDefault()
35+
}
36+
37+
/** Holds if this constant only represents TLS1.2 protocols. */
38+
predicate isTLS1_2Only() {
39+
// Flags for TLS1.2 are 0x00000400 and 0x00000800
40+
// 3072 = 0x00000400 | 0x00000800
41+
this.getBitmask().bitAnd(3072.bitNot()) = 0 and
42+
not this.isSystemDefault()
43+
}
44+
45+
/** Holds if this constant only represents TLS1.1 protocols. */
46+
predicate isTLS1_1Only() {
47+
// Flags for TLS1.1 are 0x00000100 and 0x00000200
48+
// 768 = 0x00000100 | 0x00000200
49+
this.getBitmask().bitAnd(768.bitNot()) = 0 and
50+
not this.isSystemDefault()
51+
}
52+
53+
/** Holds if this constant only represents TLS1.0 protocols. */
54+
predicate isTLS1_0Only() {
55+
// Flags for TLS1.0 are 0x00000040 and 0x00000080
56+
// 192 = 0x00000040 | 0x00000080
57+
this.getBitmask().bitAnd(192.bitNot()) = 0 and
58+
not this.isSystemDefault()
59+
}
60+
61+
/** Holds if this constant only represents TLS1.1 protocols. */
62+
predicate isSSL3Only() {
63+
// Flags for SSL3 are 0x00000010 and 0x00000020
64+
// 48 = 0x00000010 | 0x00000020
65+
this.getBitmask().bitAnd(48.bitNot()) = 0 and
66+
not this.isSystemDefault()
67+
}
68+
69+
/** Holds if this constant only represents SSL2 protocols. */
70+
predicate isSSL2Only() {
71+
// Flags for TLS1.0 are 0x00000004 and 0x00000008
72+
// 12 = 0x00000004 | 0x00000008
73+
this.getBitmask().bitAnd(12.bitNot()) = 0 and
74+
not this.isSystemDefault()
75+
}
76+
77+
/** Holds if this constant only represents PCT1 protocols. */
78+
predicate isPCT1Only() {
79+
// Flags for PCT are 0x00000001 and 0x00000002
80+
// 3 = 0x00000001 | 0x00000002
81+
this.getBitmask().bitAnd(3.bitNot()) = 0 and
82+
not this.isSystemDefault()
83+
}
84+
85+
/** Holds if this constant only represents any combination of TLS-related protocols. */
86+
predicate isHardcodedProtocol() {
87+
// 16383 = SP_PROT_TLS1_3 | SP_PROT_TLS1_2 | SP_PROT_TLS1_1 | SP_PROT_TLS1_3
88+
// | SP_PROT_TLS1 | SP_PROT_SSL3 | SP_PROT_SSL2 | SP_PROT_PCT1
89+
this.getBitmask().bitAnd(16383.bitNot()) = 0 and
90+
not this.isSystemDefault()
91+
}
92+
93+
/** Holds if this constant represents the system default protocol. */
94+
predicate isSystemDefault() { this.getBitmask() = 0 }
95+
}
96+
97+
/**
98+
* A data flow configuration that tracks from constant values to assignments to the
99+
* `grbitEnabledProtocols` field on the SCHANNEL_CRED structure.
100+
*/
101+
module GrbitEnabledConstantConfiguration implements DataFlow::ConfigSig {
102+
predicate isSource(DataFlow::Node source) { source.asExpr().isConstant() }
103+
104+
predicate isSink(DataFlow::Node sink) {
105+
exists(Field grbitEnabledProtocols |
106+
grbitEnabledProtocols.hasName("grbitEnabledProtocols") and
107+
sink.asExpr() = grbitEnabledProtocols.getAnAssignedValue()
108+
)
109+
}
110+
111+
predicate isBarrier(DataFlow::Node node) {
112+
// Do not flow through other macro invocations if they would, themselves, be represented
113+
node.asExpr() = any(ConstantMacroInvocation mi).getExpr().getAChild+()
114+
or
115+
// Do not flow through complements, as they change the meaning
116+
node.asExpr() instanceof ComplementExpr
117+
}
118+
}
119+
120+
module GrbitEnabledConstantTace = TaintTracking::Global<GrbitEnabledConstantConfiguration>;
121+
122+
/**
123+
* A macro that represents a constant value.
124+
*/
125+
class ConstantMacroInvocation extends MacroInvocation {
126+
ConstantMacroInvocation() {
127+
exists(this.getExpr().getValue()) and
128+
not this.getMacro().getHead().matches("%(%)%")
129+
}
130+
}
131+
132+
/**
133+
* Gets the name of the constant `val`, if it is a constant.
134+
*/
135+
string getConstantName(Expr val) {
136+
exists(val.getValue()) and
137+
if exists(ConstantMacroInvocation mi | mi.getExpr() = val)
138+
then result = any(ConstantMacroInvocation mi | mi.getExpr() = val).getMacroName()
139+
else result = val.toString()
140+
}

‎cpp/ql/src/Microsoft/Security/Protocols/UseOfDeprecatedSecurityProtocol.qhelp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Older protocol versions of TLS are less secure than TLS 1.2 and TLS 1.3 and are more likely to have new vulnerabilities. Avoid older protocol versions to minimize risk.</p>
8+
<p>The <code>grbitEnabledProtocols</code> member of the <code>SCHANNEL_CRED</code> struct contains a bit string that represents the protocols supported by connections made with credentials acquired by using this structure. If this member is zero, Schannel selects the protocol. Applications should set <code>grbitEnabledProtocols</code> to zero and use the protocol versions enabled on the system by default.</p>
9+
</overview>
10+
11+
<recommendation>
12+
<p> - Set the <code>grbitEnabledProtocols</code> member of the <code>SCHANNEL_CRED</code> struct to <code>0</code>.</p>
13+
</recommendation>
14+
15+
<example>
16+
<p>Violation:</p>
17+
18+
<sample src="examples/UseOfDeprecatedSecurityProtocol/UseOfDeprecatedSecurityProtocol1.cpp" />
19+
20+
<p>Solution:</p>
21+
22+
<sample src="examples/UseOfDeprecatedSecurityProtocol/UseOfDeprecatedSecurityProtocol2.cpp" />
23+
</example>
24+
25+
<references>
26+
<li>Microsoft Docs: <a href="https://learn.microsoft.com/en-us/windows/win32/api/schannel/ns-schannel-schannel_cred">SCHANNEL_CRED structure</a>.</li>
27+
</references>
28+
29+
</qhelp>

‎cpp/ql/src/Microsoft/Security/Protocols/UseOfDeprecatedSecurityProtocol.ql

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Hard-coded use of a deprecated security protocol
3+
* @description Using a deprecated security protocol rather than the system default is risky.
4+
* @kind problem
5+
* @problem.severity error
6+
* @id cpp/use-of-deprecated-security-protocol
7+
*/
8+
9+
import cpp
10+
import HardCodedSecurityProtocol
11+
12+
from ProtocolConstant constantValue, DataFlow::Node grbitEnabledProtocolsAssignment
13+
where
14+
GrbitEnabledConstantTace::flow(DataFlow::exprNode(constantValue), grbitEnabledProtocolsAssignment) and
15+
// If the system default hasn't been specified, and TLS2 has not been specified, then this is a deprecated security protocol
16+
not constantValue.isSystemDefault() and
17+
not constantValue.isTLS1_2Only() and
18+
not constantValue.isTLS1_3Only()
19+
select constantValue,
20+
"Hard-coded use of deprecated security protocol " + getConstantName(constantValue) +
21+
" set here $@.", constantValue, getConstantName(constantValue)

‎cpp/ql/src/Microsoft/Security/Protocols/examples/HardCodedSecurityProtocol/HardCodedSecurityProtocol1.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#include <windows.h>
2+
#include <ntsecapi.h>
3+
#include <stdio.h>
4+
#include <sspi.h>
5+
#include <schnlsp.h>
6+
7+
void HardCodedSecurityProtocolGood()
8+
{
9+
10+
SCHANNEL_CRED credData;
11+
ZeroMemory(&credData, sizeof(credData));
12+
13+
// BAD: hardcoded protocols
14+
credData.grbitEnabledProtocols = SP_PROT_TLS1_2;
15+
credData.grbitEnabledProtocols = SP_PROT_TLS1_3;
16+
17+
return;
18+
}

‎cpp/ql/src/Microsoft/Security/Protocols/examples/HardCodedSecurityProtocol/HardCodedSecurityProtocol2.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#include <windows.h>
2+
#include <ntsecapi.h>
3+
#include <stdio.h>
4+
#include <sspi.h>
5+
#include <schnlsp.h>
6+
7+
void HardCodedSecurityProtocolGood()
8+
{
9+
10+
SCHANNEL_CRED credData;
11+
ZeroMemory(&credData, sizeof(credData));
12+
13+
// GOOD: system default protocol
14+
credData.grbitEnabledProtocols = 0;
15+
16+
return;
17+
}

‎cpp/ql/src/Microsoft/Security/Protocols/examples/UseOfDeprecatedSecurityProtocol/UseOfDeprecatedSecurityProtocol1.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#include <windows.h>
2+
#include <ntsecapi.h>
3+
#include <stdio.h>
4+
#include <sspi.h>
5+
#include <schnlsp.h>
6+
7+
void UseOfDeprecatedSecurityProtocolGood()
8+
{
9+
10+
SCHANNEL_CRED credData;
11+
ZeroMemory(&credData, sizeof(credData));
12+
13+
// BAD: Deprecated protocols
14+
credData.grbitEnabledProtocols = SP_PROT_PCT1_SERVER;
15+
credData.grbitEnabledProtocols = SP_PROT_SSL2_SERVER;
16+
credData.grbitEnabledProtocols = SP_PROT_SSL3_SERVER;
17+
credData.grbitEnabledProtocols = SP_PROT_TLS1_1;
18+
credData.grbitEnabledProtocols = SP_PROT_TLS1_1_SERVER;
19+
credData.grbitEnabledProtocols = SP_PROT_TLS1_1_CLIENT;
20+
credData.grbitEnabledProtocols = SP_PROT_SSL3TLS1;
21+
22+
return;
23+
}

‎cpp/ql/src/Microsoft/Security/Protocols/examples/UseOfDeprecatedSecurityProtocol/UseOfDeprecatedSecurityProtocol2.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#include <windows.h>
2+
#include <ntsecapi.h>
3+
#include <stdio.h>
4+
#include <sspi.h>
5+
#include <schnlsp.h>
6+
7+
void HardCodedSecurityProtocolGood()
8+
{
9+
10+
SCHANNEL_CRED credData;
11+
ZeroMemory(&credData, sizeof(credData));
12+
13+
// GOOD: system default protocol
14+
credData.grbitEnabledProtocols = 0;
15+
16+
return;
17+
}

0 commit comments

Comments
 (0)