10000 Decouple UnsafeCertTrust.qll to reuse the taint tracking configuration · github/codeql@ba42105 · GitHub
[go: up one dir, main page]
Skip to content

Commit ba42105

Browse files
atorralbaatorralba
committed
Decouple UnsafeCertTrust.qll to reuse the taint tracking configuration
1 parent 3e0a3f0 commit ba42105

File tree

5 files changed

+68
-73
lines changed

5 files changed

+68
-73
lines changed

‎java/change-notes/2021-06-28-unsafe-cert-trust-query.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
lgtm,codescanning
2-
* The query "Unsafe certificate trust" (`java/unsafe-cert-trust`) has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @luchua-bc](https://github.com/github/codeql/pull/3550)
2+
* The query "Unsafe certificate trust" (`java/unsafe-cert-trust`) has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @luchua-bc](https://github.com/github/codeql/pull/3550).

‎java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.ql

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,7 @@
1212
*/
1313

1414
import java
15-
import semmle.code.java.dataflow.TaintTracking
16-
import semmle.code.java.security.UnsafeCertTrust
17-
18-
class SslEndpointIdentificationFlowConfig extends TaintTracking::Configuration {
19-
SslEndpointIdentificationFlowConfig() { this = "SslEndpointIdentificationFlowConfig" }
20-
21-
override predicate isSource(DataFlow::Node source) { source instanceof SslConnectionInit }
22-
23-
override predicate isSink(DataFlow::Node sink) { sink instanceof SslConnectionCreation }
24-
25-
override predicate isSanitizer(DataFlow::Node sanitizer) {
26-
sanitizer instanceof SslUnsafeCertTrustSanitizer
27-
}
28-
}
15+
import semmle.code.java.security.UnsafeCertTrustQuery
2916

3017
from Expr unsafeTrust
3118
where

‎java/ql/src/semmle/code/java/security/UnsafeCertTrust.qll

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import java
44
private import semmle.code.java.frameworks.Networking
55
private import semmle.code.java.security.Encryption
66
private import semmle.code.java.dataflow.DataFlow
7-
private import semmle.code.java.dataflow.DataFlow2
87

98
/**
109
* The creation of an object that prepares an SSL connection.
@@ -50,19 +49,6 @@ class SslConnectionCreation extends DataFlow::Node {
5049
*/
5150
abstract class SslUnsafeCertTrustSanitizer extends DataFlow::Node { }
5251

53-
/**
54-
* An SSL object that was assigned a safe `SSLParameters` object and can be considered safe.
55-
*/
56-
private class SslConnectionWithSafeSslParameters extends SslUnsafeCertTrustSanitizer {
57-
SslConnectionWithSafeSslParameters() {
58-
exists(SafeSslParametersFlowConfig config, DataFlow::Node safe, DataFlow::Node sanitizer |
59-
config.hasFlowTo(safe) and
60-
sanitizer = DataFlow::exprNode(safe.asExpr().(Argument).getCall().getQualifier()) and
61-
DataFlow::localFlow(sanitizer, this)
62-
)
63-
}
64-
}
65-
6652
/**
6753
* An `SSLEngine` set in server mode.
6854
*/
@@ -92,34 +78,6 @@ private predicate isSslSocket(MethodAccess createSocket) {
9278
createSocket.getQualifier().getType().(RefType).getASupertype*() instanceof SSLSocketFactory
9379
}
9480

95-
private class SafeSslParametersFlowConfig extends DataFlow2::Configuration {
96-
SafeSslParametersFlowConfig() { this = "SafeSslParametersFlowConfig" }
97-
98-
override predicate isSource(DataFlow::Node source) {
99-
exists(MethodAccess ma |
100-
ma instanceof SafeSetEndpointIdentificationAlgorithm and
101-
DataFlow::getInstanceArgument(ma) = source.(DataFlow::PostUpdateNode).getPreUpdateNode()
102-
)
103-
}
104-
105-
override predicate isSink(DataFlow::Node sink) {
106-
exists(MethodAccess ma, RefType t | t instanceof SSLSocket or t instanceof SSLEngine |
107-
ma.getMethod().hasName("setSSLParameters") and
108-
ma.getMethod().getDeclaringType().getASupertype*() = t and
109-
ma.getArgument(0) = sink.asExpr()
110-
)
111-
}
112-
}
113-
114-
private class SafeSetEndpointIdentificationAlgorithm extends MethodAccess {
115-
SafeSetEndpointIdentificationAlgorithm() {
116-
this.getMethod().hasName("setEndpointIdentificationAlgorithm") and
117-
this.getMethod().getDeclaringType() instanceof SSLParameters and
118-
not this.getArgument(0) instanceof NullLiteral and
119-
not this.getArgument(0).(CompileTimeConstantExpr).getStringValue().length() = 0
120-
}
121-
}
122-
12381
/**
12482
* A call to a method that enables SSL (`useSslProtocol` or `setSslContextFactory`)
12583
* on an instance of `com.rabbitmq.client.ConnectionFactory` that doesn't set `enableHostnameVerification`.

‎java/ql/src/semmle/code/java/security/UnsafeCertTrustQuery.qll

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/** Provides taint tracking configurations to be used by unsafe certificate trust queries. */
2+
3+
import java
4+
import semmle.code.java.dataflow.TaintTracking
5+
import semmle.code.java.security.UnsafeCertTrust
6+
import semmle.code.java.security.Encryption
7+
8+
class SslEndpointIdentificationFlowConfig extends TaintTracking::Configuration {
9+
SslEndpointIdentificationFlowConfig() { this = "SslEndpointIdentificationFlowConfig" }
10+
11+
override predicate isSource(DataFlow::Node source) { source instanceof SslConnectionInit }
12+
13+
override predicate isSink(DataFlow::Node sink) { sink instanceof SslConnectionCreation }
14+
15+
override predicate isSanitizer(DataFlow::Node sanitizer) {
16+
sanitizer instanceof SslUnsafeCertTrustSanitizer
17+
}
18+
}
19+
20+
/**
21+
* An SSL object that was assigned a safe `SSLParameters` object and can be considered safe.
22+
*/
23+
private class SslConnectionWithSafeSslParameters extends SslUnsafeCertTrustSanitizer {
24+
SslConnectionWithSafeSslParameters() {
25+
exists(SafeSslParametersFlowConfig config, DataFlow::Node safe, DataFlow::Node sanitizer |
26+
config.hasFlowTo(safe) and
27+
sanitizer = DataFlow::exprNode(safe.asExpr().(Argument).getCall().getQualifier()) and
28+
DataFlow::localFlow(sanitizer, this)
29+
)
30+
}
31+
}
32+
33+
private class SafeSslParametersFlowConfig extends DataFlow2::Configuration {
34+
SafeSslParametersFlowConfig() { this = "SafeSslParametersFlowConfig" }
35+
36+
override predicate isSource(DataFlow::Node source) {
37+
exists(MethodAccess ma |
38+
ma instanceof SafeSetEndpointIdentificationAlgorithm and
39+
DataFlow::getInstanceArgument(ma) = source.(DataFlow::PostUpdateNode).getPreUpdateNode()
40+
)
41+
}
42+
43+
override predicate isSink(DataFlow::Node sink) {
44+
exists(MethodAccess ma, RefType t | t instanceof SSLSocket or t instanceof SSLEngine |
45+
ma.getMethod().hasName("setSSLParameters") and
46+
ma.getMethod().getDeclaringType().getASupertype*() = t and
47+
ma.getArgument(0) = sink.asExpr()
48+
)
49+
}
50+
}
51+
52+
/**
53+
* A call to `SSLParameters.setEndpointIdentificationAlgorithm` with a non-null and non-zero parameter.
54+
*/
55+
private class SafeSetEndpointIdentificationAlgorithm extends MethodAccess {
56+
SafeSetEndpointIdentificationAlgorithm() {
57+
this.getMethod().hasName("setEndpointIdentificationAlgorithm") and
58+
this.getMethod().getDeclaringType() instanceof SSLParameters and
59+
not this.getArgument(0) instanceof NullLiteral and
60+
not this.getArgument(0).(CompileTimeConstantExpr).getStringValue().length() = 0
61+
}
62+
}

‎java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.ql

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,7 @@
11
import java
2-
import semmle.code.java.dataflow.FlowSources
3-
import semmle.code.java.dataflow.TaintTracking
4-
import semmle.code.java.security.UnsafeCertTrust
2+
import semmle.code.java.security.UnsafeCertTrustQuery
53
import TestUtilities.InlineExpectationsTest
64

7-
class Conf extends TaintTracking::Configuration {
8-
Conf() { this = "qltest:cwe:unsafe-cert-trust" }
9-
10-
override predicate isSource(DataFlow::Node source) { source instanceof SslConnectionInit }
11-
12-
override predicate isSink(DataFlow::Node sink) { sink instanceof SslConnectionCreation }
13-
14-
override predicate isSanitizer(DataFlow::Node sanitizer) {
15-
sanitizer instanceof SslUnsafeCertTrustSanitizer
16-
}
17-
}
18-
195
class UnsafeCertTrustTest extends InlineExpectationsTest {
206
UnsafeCertTrustTest() { this = "HasUnsafeCertTrustTest" }
217

@@ -26,7 +12,9 @@ class UnsafeCertTrustTest extends InlineExpectationsTest {
2612
exists(Expr unsafeTrust |
2713
unsafeTrust instanceof RabbitMQEnableHostnameVerificationNotSet
2814
or
29-
exists(Conf config | config.hasFlowTo(DataFlow::exprNode(unsafeTrust)))
15+
exists(SslEndpointIdentificationFlowConfig config |
16+
config.hasFlowTo(DataFlow::exprNode(unsafeTrust))
17+
)
3018
|
3119
unsafeTrust.getLocation() = location and
3220
element = unsafeTrust.toString() and

0 commit comments

Comments
 (0)
0