Skip to content

Commit ab4dc30

Browse files
atorralbaatorralba
committed
Refactor into libraries
1 parent 7cd05fb commit ab4dc30

File tree

4 files changed

+141
-132
lines changed

4 files changed

+141
-132
lines changed

‎java/ql/lib/semmle/code/java/security/InsecureTrustManager.qll‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import java
2+
private import semmle.code.java.controlflow.Guards
3+
private import semmle.code.java.security.Encryption
4+
private import semmle.code.java.security.SecurityFlag
5+
6+
/** The creation of an insecure `TrustManager`. */
7+
abstract class InsecureTrustManagerSource extends DataFlow::Node { }
8+
9+
private class DefaultInsecureTrustManagerSource extends InsecureTrustManagerSource {
10+
DefaultInsecureTrustManagerSource() {
11+
this.asExpr().(ClassInstanceExpr).getConstructedType() instanceof InsecureX509TrustManager
12+
}
13+
}
14+
15+
/**
16+
* The use of a `TrustManager` in an SSL context.
17+
* Intentionally insecure connections are not considered sinks.
18+
*/
19+
abstract class InsecureTrustManagerSink extends DataFlow::Node {
20+
InsecureTrustManagerSink() { not isGuardedByInsecureFlag(this) }
21+
}
22+
23+
private class DefaultInsecureTrustManagerSink extends InsecureTrustManagerSink {
24+
DefaultInsecureTrustManagerSink() {
25+
exists(MethodAccess ma, Method m |
26+
m.hasName("init") and
27+
m.getDeclaringType() instanceof SSLContext and
28+
ma.getMethod() = m
29+
|
30+
ma.getArgument(1) = this.asExpr()
31+
)
32+
}
33+
}
34+
35+
/** Holds if `node` is guarded by a flag that suggests an intentionally insecure use. */
36+
private predicate isGuardedByInsecureFlag(DataFlow::Node node) {
37+
exists(Guard g | g.controls(node.asExpr().getBasicBlock(), _) |
38+
g = getASecurityFeatureFlagGuard() or g = getAnInsecureTrustManagerFlagGuard()
39+
)
40+
}
41+
42+
/**
43+
* An insecure `X509TrustManager`.
44+
* An `X509TrustManager` is considered insecure if it never throws a `CertificateException`
45+
* and therefore implicitly trusts any certificate as valid.
46+
*/
47+
private class InsecureX509TrustManager extends RefType {
48+
InsecureX509TrustManager() {
49+
this.getASupertype*() instanceof X509TrustManager and
50+
exists(Method m |
51+
m.getDeclaringType() = this and
52+
m.hasName("checkServerTrusted") and
53+
not mayThrowCertificateException(m)
54+
)
55+
}
56+
}
57+
58+
/** The `java.security.cert.CertificateException` class. */
59+
private class CertificateException extends RefType {
60+
CertificateException() { this.hasQualifiedName("java.security.cert", "CertificateException") }
61+
}
62+
63+
/**
64+
* Holds if:
65+
* - `m` may `throw` a `CertificateException`, or
66+
* - `m` calls another method that may throw, or
67+
* - `m` calls a method declared to throw a `CertificateException`, but for which no source is available
68+
*/
69+
private predicate mayThrowCertificateException(Method m) {
70+
exists(ThrowStmt throwStmt |
71+
throwStmt.getThrownExceptionType().getASupertype*() instanceof CertificateException
72+
|
73+
throwStmt.getEnclosingCallable() = m
74+
)
75+
or
76+
exists(Method otherMethod | m.polyCalls(otherMethod) |
77+
mayThrowCertificateException(otherMethod)
78+
or
79+
not otherMethod.fromSource() and
80+
otherMethod.getAnException().getType().getASupertype*() instanceof CertificateException
81+
)
82+
}
83+
84+
/**
85+
* Flags suggesting a deliberately insecure `TrustManager` usage.
86+
*/
87+
private class InsecureTrustManagerFlag extends FlagKind {
88+
InsecureTrustManagerFlag() { this = "InsecureTrustManagerFlag" }
89+
90+
bindingset[result]
91+
override string getAFlagName() {
92+
result
93+
.regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*") and
94+
result != "equalsIgnoreCase"
95+
}
96+
}
97+
98+
/** Gets a guard that represents a (likely) flag controlling an insecure `TrustManager` use. */
99+
private Guard getAnInsecureTrustManagerFlagGuard() {
100+
result = any(InsecureTrustManagerFlag flag).getAFlag().asExpr()
101+
}

‎java/ql/lib/semmle/code/java/security/InsecureTrustManagerQuery.qll‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/** Provides taint tracking configurations to be used in Trust Manager queries. */
2+
3+
import java
4+
import semmle.code.java.dataflow.FlowSources
5+
import semmle.code.java.security.Encryption
6+
import semmle.code.java.security.InsecureTrustManager
7+
8+
/**
9+
* A configuration to model the flow of an insecure `TrustManager`
10+
* to the initialization of an SSL context.
11+
*/
12+
class InsecureTrustManagerConfiguration extends TaintTracking::Configuration {
13+
InsecureTrustManagerConfiguration() { this = "InsecureTrustManagerConfiguration" }
14+
15+
override predicate isSource(DataFlow::Node source) {
16+
source instanceof InsecureTrustManagerSource
17+
}
18+
19+
override predicate isSink(DataFlow::Node sink) { sink instanceof InsecureTrustManagerSink }
20+
}

‎java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql‎

Lines changed: 5 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -10,108 +10,11 @@
1010
*/
1111

1212
import java
13-
import semmle.code.java.controlflow.Guards
1413
import semmle.code.java.dataflow.DataFlow
15-
import semmle.code.java.dataflow.FlowSources
16-
import semmle.code.java.security.Encryption
17-
import semmle.code.java.security.SecurityFlag
14+
import semmle.code.java.security.InsecureTrustManagerQuery
1815
import DataFlow::PathGraph
1916

20-
/**
21-
* An insecure `X509TrustManager`.
22-
* An `X509TrustManager` is considered insecure if it never throws a `CertificateException`
23-
* and therefore implicitly trusts any certificate as valid.
24-
*/
25-
class InsecureX509TrustManager extends RefType {
26-
InsecureX509TrustManager() {
27-
this.getASupertype*() instanceof X509TrustManager and
28-
exists(Method m |
29-
m.getDeclaringType() = this and
30-
m.hasName("checkServerTrusted") and
31-
not mayThrowCertificateException(m)
32-
)
33-
}
34-
}
35-
36-
/** The `java.security.cert.CertificateException` class. */
37-
private class CertificateException extends RefType {
38-
CertificateException() { this.hasQualifiedName("java.security.cert", "CertificateException") }
39-
}
40-
41-
/**
42-
* Holds if:
43-
* - `m` may `throw` a `CertificateException`, or
44-
* - `m` calls another method that may throw, or
45-
* - `m` calls a method declared to throw a `CertificateException`, but for which no source is available
46-
*/
47-
private predicate mayThrowCertificateException(Method m) {
48-
exists(ThrowStmt throwStmt |
49-
throwStmt.getThrownExceptionType().getASupertype*() instanceof CertificateException
50-
|
51-
throwStmt.getEnclosingCallable() = m
52-
)
53-
or
54-
exists(Method otherMethod | m.polyCalls(otherMethod) |
55-
mayThrowCertificateException(otherMethod)
56-
or
57-
not otherMethod.fromSource() and
58-
otherMethod.getAnException().getType().getASupertype*() instanceof CertificateException
59-
)
60-
}
61-
62-
/**
63-
* A configuration to model the flow of an `InsecureX509TrustManager` to an `SSLContext.init` call.
64-
*/
65-
class InsecureTrustManagerConfiguration extends TaintTracking::Configuration {
66-
InsecureTrustManagerConfiguration() { this = "InsecureTrustManagerConfiguration" }
67-
68-
override predicate isSource(DataFlow::Node source) {
69-
source.asExpr().(ClassInstanceExpr).getConstructedType() instanceof InsecureX509TrustManager
70-
}
71-
72-
override predicate isSink(DataFlow::Node sink) {
73-
exists(MethodAccess ma, Method m |
74-
m.hasName("init") and
75-
m.getDeclaringType() instanceof SSLContext and
76-
ma.getMethod() = m
77-
|
78-
ma.getArgument(1) = sink.asExpr()
79-
)
80-
}
81-
}
82-
83-
/**
84-
* Flags suggesting a deliberately insecure `TrustManager` usage.
85-
*/
86-
private class InsecureTrustManagerFlag extends FlagKind {
87-
InsecureTrustManagerFlag() { this = "InsecureTrustManagerFlag" }
88-
89-
bindingset[result]
90-
override string getAFlagName() {
91-
result
92-
.regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*") and
93-
result != "equalsIgnoreCase"
94-
}
95-
}
96-
97-
/** Gets a guard that represents a (likely) flag controlling an insecure `TrustManager` use. */
98-
private Guard getAnInsecureTrustManagerFlagGuard() {
99-
result = any(InsecureTrustManagerFlag flag).getAFlag().asExpr()
100-
}
101-
102-
/** Holds if `node` is guarded by a flag that suggests an intentionally insecure use. */
103-
private predicate isNodeGuardedByFlag(DataFlow::Node node) {
104-
exists(Guard g | g.controls(node.asExpr().getBasicBlock(), _) |
105-
g = getASecurityFeatureFlagGuard() or g = getAnInsecureTrustManagerFlagGuard()
106-
)
107-
}
108-
109-
from
110-
DataFlow::PathNode source, DataFlow::PathNode sink, InsecureTrustManagerConfiguration cfg,
111-
RefType trustManager
112-
where
113-
cfg.hasFlowPath(source, sink) and
114-
not isNodeGuardedByFlag(sink.getNode()) and
115-
trustManager = source.getNode().asExpr().(ClassInstanceExpr).getConstructedType()
116-
select sink, source, sink, "$@ that is defined $@ and trusts any certificate, is used here.",
117-
source, "This trustmanager", trustManager, "here"
17+
from DataFlow::PathNode source, DataFlow::PathNode sink
18+
where any(InsecureTrustManagerConfiguration cfg).hasFlowPath(source, sink)
19+
select sink, source, sink, "This $@, that is defined $@, trusts any certificate and is used here.",
20+
source, "TrustManager", source.getNode().asExpr().(ClassInstanceExpr).getConstructedType(), "here"

0 commit comments

Comments
 (0)