Skip to content

Commit 5eb814f

Browse files
geoffw0geoffw0
committed
C++: Prototype SSL result not checked query.
1 parent 1f3f7e9 commit 5eb814f

File tree

4 files changed

+270
-0
lines changed

4 files changed

+270
-0
lines changed

‎cpp/ql/src/Security/CWE/CWE-295/SSLResultNotChecked.ql

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/**
2+
* @name TODO
3+
* @description TODO
4+
* @kind problem
5+
* @problem.severity TODO
6+
* @security-severity TODO
7+
* @precision TODO
8+
* @id TODO
9+
* @tags TODO
10+
*/
11+
12+
13+
import cpp
14+
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
15+
//import semmle.code.cpp.controlflow.Guards
16+
import semmle.code.cpp.controlflow.IRGuards
17+
18+
/**
19+
* A call to `SSL_get_peer_certificate`.
20+
*/
21+
class SSLGetPeerCertificateCall extends FunctionCall {
22+
SSLGetPeerCertificateCall() {
23+
getTarget().getName() = "SSL_get_peer_certificate" // SSL_get_peer_certificate(ssl)
24+
}
25+
26+
// TODO: getSSLArg?
27+
}
28+
29+
/**
30+
* A call to `SSL_get_verify_result`.
31+
*/
32+
class SSLGetVerifyResultCall extends FunctionCall {
33+
SSLGetVerifyResultCall() {
34+
getTarget().getName() = "SSL_get_verify_result" // SSL_get_peer_certificate(ssl)
35+
}
36+
}
37+
38+
/**
39+
* Holds if the SSL object passed into `SSL_get_peer_certificate` is checked with
40+
* `SSL_get_verify_result` entering `node`.
41+
*/
42+
predicate resultIsChecked(SSLGetPeerCertificateCall getCertCall, ControlFlowNode node) {
43+
exists(Expr ssl, SSLGetVerifyResultCall check |
44+
ssl = globalValueNumber(getCertCall.getArgument(0)).getAnExpr() and
45+
ssl = check.getArgument(0) and
46+
node = check
47+
)
48+
}
49+
50+
/**
51+
* Holds if the certificate returned by `SSL_get_peer_certificate` is found to be
52+
* `0` on the edge `node1` to `node2`.
53+
*/
54+
predicate certIsZero(SSLGetPeerCertificateCall getCertCall, ControlFlowNode node1, ControlFlowNode node2) {
55+
exists(GuardCondition guard, Expr cert |
56+
cert = globalValueNumber(getCertCall).getAnExpr() and
57+
(
58+
exists(Expr zero |
59+
zero.getValue().toInt() = 0 and
60+
node1 = guard and
61+
(
62+
(
63+
guard.comparesEq(cert, zero, 0, true, true) and // if (cert == zero) {
64+
node2 = guard.getATrueSuccessor()
65+
) or (
66+
guard.comparesEq(cert, zero, 0, false, true) and // if (cert != zero) { }
67+
node2 = guard.getAFalseSuccessor()
68+
)
69+
)
70+
) or (
71+
guard = cert and // if (cert) { }
72+
node1 = guard and
73+
node2 = guard.getAFalseSuccessor()
74+
) or (
75+
node1 = guard.getParent() and
76+
node2 = guard.getParent().(NotExpr).getATrueSuccessor() // if (!cert) {
77+
)
78+
)
79+
)
80+
}
81+
82+
/**
83+
* Holds if the SSL object passed into `SSL_get_peer_certificate` has not been checked with
84+
* `SSL_get_verify_result` at `node`. Note that this is only computed at the call to
85+
* `SSL_get_peer_certificate` and at the start and end of `BasicBlock`s.
86+
*/
87+
predicate certNotChecked(SSLGetPeerCertificateCall getCertCall, ControlFlowNode node) {
88+
(
89+
// cert is not checked at the call to `SSL_get_peer_certificate`
90+
node = getCertCall
91+
) or exists(BasicBlock bb, int pos |
92+
// flow to end of a `BasicBlock`
93+
certNotChecked(getCertCall, bb.getNode(pos)) and
94+
node = bb.getEnd() and
95+
96+
// check for barrier node
97+
not exists(int pos2 |
98+
pos2 > pos and
99+
resultIsChecked(getCertCall, bb.getNode(pos2))
100+
)
101+
) or exists(BasicBlock pred, BasicBlock bb |
102+
// flow from the end of one `BasicBlock` to the beginning of a successor
103+
certNotChecked(getCertCall, pred.getEnd()) and
104+
bb = pred.getASuccessor() and
105+
node = bb.getStart() and
106+
107+
// check for barrier bb
108+
not certIsZero(getCertCall, pred.getEnd(), bb.getStart())
109+
)
110+
}
111+
112+
from
113+
SSLGetPeerCertificateCall getCertCall, ControlFlowNode node
114+
where
115+
certNotChecked(getCertCall, node) and
116+
node instanceof Function // (function exit)
117+
select
118+
getCertCall, "This " + getCertCall.toString() + " is not followed by a call to SSL_get_verify_result."

‎cpp/ql/test/query-tests/Security/CWE/CWE-295/SSLResultNotChecked.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| test2.cpp:13:13:13:36 | call to SSL_get_peer_certificate | This call to SSL_get_peer_certificate is not followed by a call to SSL_get_verify_result. |
2+
| test2.cpp:28:13:28:36 | call to SSL_get_peer_certificate | This call to SSL_get_peer_certificate is not followed by a call to SSL_get_verify_result. |
3+
| test2.cpp:61:9:61:32 | call to SSL_get_peer_certificate | This call to SSL_get_peer_certificate is not followed by a call to SSL_get_verify_result. |
4+
| test2.cpp:89:9:89:32 | call to SSL_get_peer_certificate | This call to SSL_get_peer_certificate is not followed by a call to SSL_get_verify_result. |

‎cpp/ql/test/query-tests/Security/CWE/CWE-295/SSLResultNotChecked.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-295/SSLResultNotChecked.ql

‎cpp/ql/test/query-tests/Security/CWE/CWE-295/test2.cpp

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
2+
struct SSL {
3+
// ...
4+
};
5+
6+
int SSL_get_peer_certificate(const SSL *ssl);
7+
int SSL_get_verify_result(const SSL *ssl);
8+
9+
bool maybe();
10+
11+
bool test2_1(SSL *ssl)
12+
{
13+
int cert = SSL_get_peer_certificate(ssl); // BAD (SSL_get_verify_result is never called)
14+
15+
return true;
16+
}
17+
18+
bool test2_2(SSL *ssl)
19+
{
20+
int cert = SSL_get_peer_certificate(ssl); // GOOD (SSL_get_verify_result is always called)
21+
int result = SSL_get_verify_result(ssl);
22+
23+
return (result == 0);
24+
}
25+
26+
bool test2_3(SSL *ssl)
27+
{
28+
int cert = SSL_get_peer_certificate(ssl); // BAD (SSL_get_verify_result may not be called)
29+
30+
if (maybe())
31+
{
32+
int result = SSL_get_verify_result(ssl);
33+
34+
return (result == 0);
35+
}
36+
37+
return true;
38+
}
39+
40+
bool test2_4(SSL *ssl)
41+
{
42+
int cert, result;
43+
44+
cert = SSL_get_peer_certificate(ssl); // GOOD (SSL_get_verify_result is called when there is a cert)
45+
if (cert != 0)
46+
{
47+
result = SSL_get_verify_result(ssl);
48+
if (result == 0)
49+
{
50+
return true;
51+
}
52+
}
53+
54+
return false;
55+
}
56+
57+
bool test2_5(SSL *ssl)
58+
{
59+
int cert, result;
60+
61+
cert = SSL_get_peer_certificate(ssl); // BAD (SSL_get_verify_result is not used reliably)
62+
if ((cert != 0) && (maybe()))
63+
{
64+
result = SSL_get_verify_result(ssl);
65+
if (result == 0)
66+
{
67+
return true;
68+
}
69+
}
70+
71+
return false;
72+
}
73+
74+
bool test2_6(SSL *ssl)
75+
{
76+
int cert;
77+
78+
cert = SSL_get_peer_certificate(ssl); // GOOD (SSL_get_verify_result is called when there is a cert)
79+
if (cert == 0) return false;
80+
if (SSL_get_verify_result(ssl) != 0) return false;
81+
82+
return true;
83+
}
84+
85+
bool test2_7(SSL *ssl)
86+
{
87+
int cert;
88+
89+
cert = SSL_get_peer_certificate(ssl); // BAD (SSL_get_verify_result is only called when there is not a cert)
90+
if (cert != 0) return false;
91+
if (SSL_get_verify_result(ssl) != 0) return false;
92+
93+
return true;
94+
}
95+
96+
bool test2_8(SSL *ssl)
97+
{
98+
int cert;
99+
100+
cert = SSL_get_peer_certificate(ssl); // GOOD (SSL_get_verify_result is called when there is a cert) [FALSE POSITIVE]
101+
if (!cert) return false;
102+
if (!SSL_get_verify_result(ssl)) return false;
103+
104+
return true;
105+
}
106+
107+
bool test2_9(SSL *ssl)
108+
{
109+
int cert;
110+
111+
cert = SSL_get_peer_certificate(ssl); // GOOD (SSL_get_verify_result is called when there is a cert) [FALSE POSITIVE]
112+
if ((!cert) || (SSL_get_verify_result(ssl) != 0)) {
113+
return false;
114+
}
115+
116+
return true;
117+
}
118+
119+
bool test2_10(SSL *ssl)
120+
{
121+
int cert = SSL_get_peer_certificate(ssl); // GOOD (SSL_get_verify_result is called when there is a cert)
122+
123+
if (cert)
124+
{
125+
int result = SSL_get_verify_result(ssl);
126+
127+
if (result == 0)
128+
{
129+
return true;
130+
}
131+
}
132+
133+
return true;
134+
}
135+
136+
bool test2_11(SSL *ssl)
137+
{
138+
int cert;
139+
140+
cert = SSL_get_peer_certificate(ssl); // GOOD (SSL_get_verify_result is called when there is a cert) [FALSE POSITIVE]
141+
142+
if ((cert) && (SSL_get_verify_result(ssl) == 0)) {
143+
return true;
144+
}
145+
146+
return false;
147+
}

0 commit comments

Comments
 (0)