Skip to content

Commit 4313baf

Browse files
atorralbaatorralba
committed
Big refactor:
- Move classes and predicates to appropriate libraries - Overhaul the endpoint identification algorithm logic to use taint tracking - Adapt tests
1 parent e0f4c73 commit 4313baf

File tree

7 files changed

+494
-169
lines changed

7 files changed

+494
-169
lines changed

‎java/ql/lib/semmle/code/java/frameworks/Networking.qll

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ class TypeSocket extends RefType {
1414
TypeSocket() { this.hasQualifiedName("java.net", "Socket") }
1515
}
1616

17+
/** The type `javax.net.SocketFactory` */
18+
class TypeSocketFactory extends RefType {
19+
TypeSocketFactory() { this.hasQualifiedName("javax.net", "SocketFactory") }
20+
}
21+
1722
/** The type `java.net.URL`. */
1823
class TypeUrl extends RefType {
1924
TypeUrl() { this.hasQualifiedName("java.net", "URL") }
@@ -143,6 +148,21 @@ class UrlOpenConnectionMethod extends Method {
143148
}
144149
}
145150

151+
/** The method `javax.net.SocketFactory::createSocket`. */
152+
class CreateSocketMethod extends Method {
153+
CreateSocketMethod() {
154+
this.hasName("createSocket") and
155+
this.getDeclaringType() instanceof TypeSocketFactory
156+
}
157+
}
158+
159+
class SocketConnectMethod extends Method {
160+
SocketConnectMethod() {
161+
this.hasName("connect") and
162+
this.getDeclaringType() instanceof TypeSocket
163+
}
164+
}
165+
146166
/**
147167
* A string matching private host names of IPv4 and IPv6, which only matches the host portion therefore checking for port is not necessary.
148168
* Several examples are localhost, reserved IPv4 IP addresses including 127.0.0.1, 10.x.x.x, 172.16.x,x, 192.168.x,x, and reserved IPv6 addresses including [0:0:0:0:0:0:0:1] and [::1]

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

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,19 @@ class SSLSession extends RefType {
3434
SSLSession() { this.hasQualifiedName("javax.net.ssl", "SSLSession") }
3535
}
3636

37+
class SSLEngine extends RefType {
38+
SSLEngine() { this.hasQualifiedName("javax.net.ssl", "SSLEngine") }
39+
}
40+
41+
class SSLSocket extends RefType {
42+
SSLSocket() { this.hasQualifiedName("javax.net.ssl", "SSLSocket") }
43+
}
44+
45+
/** The `javax.net.ssl.SSLParameters` class. */
46+
class SSLParameters extends RefType {
47+
SSLParameters() { this.hasQualifiedName("javax.net.ssl", "SSLParameters") }
48+
}
49+
3750
class HostnameVerifier extends RefType {
3851
HostnameVerifier() { this.hasQualifiedName("javax.net.ssl", "HostnameVerifier") }
3952
}
@@ -79,6 +92,14 @@ class GetSocketFactory extends Method {
7992
}
8093
}
8194

95+
/** The `createSSLEngine` method of the class `javax.net.ssl.SSLContext` */
96+
class CreateSslEngineMethod extends Method {
97+
CreateSslEngineMethod() {
98+
this.hasName("createSSLEngine") and
99+
this.getDeclaringType() instanceof SSLContext
100+
}
101+
}
102+
82103
class SetConnectionFactoryMethod extends Method {
83104
SetConnectionFactoryMethod() {
84105
this.hasName("setSSLSocketFactory") and
@@ -101,6 +122,14 @@ class SetDefaultHostnameVerifierMethod extends Method {
101122
}
102123
}
103124

125+
/** The `getSession` method of the class `javax.net.ssl.SSLSession`.select */
126+
class GetSslSessionMethod extends Method {
127+
GetSslSessionMethod() {
128+
hasName("getSession") and
129+
getDeclaringType().getASupertype*() instanceof SSLSession
130+
}
131+
}
132+
104133
bindingset[algorithmString]
105134
private string algorithmRegex(string algorithmString) {
106135
// Algorithms usually appear in names surrounded by characters that are not
@@ -168,7 +197,7 @@ string getInsecureAlgorithmRegex() {
168197
string getASecureAlgorithmName() {
169198
result =
170199
[
171-
"RSA", "SHA256", "SHA512", "CCM", "GCM", "AES(?![^a-zA-Z](ECB|CBC/PKCS[57]Padding))",
200+
"RSA", "SHA256", "SHA512", "CCM", "GCM", "AES([^a-zA-Z](?!ECB|CBC/PKCS[57]Padding)).*",
172201
"Blowfish", "ECIES"
173202
]
174203
}

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

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/** Provides classes and predicates to reason about unsafe certificate trust vulnerablities. */
2+
3+
import java
4+
private import semmle.code.java.frameworks.Networking
5+
private import semmle.code.java.security.Encryption
6+
private import semmle.code.java.dataflow.DataFlow
7+
private import semmle.code.java.dataflow.DataFlow2
8+
9+
/**
10+
* The creation of an object that prepares an SSL connection.
11+
*/
12+
class SslConnectionInit extends DataFlow::Node {
13+
SslConnectionInit() {
14+
this.asExpr().(MethodAccess).getMethod() instanceof CreateSslEngineMethod or
15+
this.asExpr().(MethodAccess).getMethod() instanceof CreateSocketMethod
16+
}
17+
}
18+
19+
/**
20+
* A call to a method that establishes an SSL connection.
21+
*/
22+
class SslConnectionCreation extends DataFlow::Node {
23+
SslConnectionCreation() {
24+
exists(MethodAccess ma, Method m |
25+
m instanceof GetSslSessionMethod or
26+
m instanceof SocketConnectMethod
27+
|
28+
ma.getMethod() = m and
29+
this.asExpr() = ma.getQualifier()
30+
)
31+
or
32+
// calls to SocketFactory.createSocket with parameters immediately create the connection
33+
exists(MethodAccess ma, Method m |
34+
ma.getMethod() = m and
35+
m instanceof CreateSocket and
36+
m.getNumberOfParameters() > 0
37+
|
38+
this.asExpr() = ma
39+
)
40+
}
41+
}
42+
43+
/**
44+
* An SSL object that was assigned a safe `SSLParameters` object an can be considered safe.
45+
*/
46+
class SslConnectionWithSafeSslParameters extends Expr {
47+
SslConnectionWithSafeSslParameters() {
48+
exists(SafeSslParametersFlowConfig config, DataFlow::Node safe |
49+
config.hasFlowTo(safe) and this = safe.asExpr().(Argument).getCall().getQualifier()
50+
)
51+
}
52+
}
53+
54+
private class SafeSslParametersFlowConfig extends DataFlow2::Configuration {
55+
SafeSslParametersFlowConfig() { this = "SafeSslParametersFlowConfig" }
56+
57+
override predicate isSource(DataFlow::Node source) {
58+
exists(MethodAccess ma |
59+
ma instanceof SafeSetEndpointIdentificationAlgorithm and
60+
ma.getQualifier() = source.asExpr()
61+
)
62+
}
63+
64+
override predicate isSink(DataFlow::Node sink) {
65+
exists(MethodAccess ma, RefType t | t instanceof SSLSocket or t instanceof SSLEngine |
66+
ma.getMethod().hasName("setSSLParameters") and
67+
ma.getMethod().getDeclaringType().getASupertype*() = t and
68+
ma.getArgument(0) = sink.asExpr()
69+
)
70+
}
71+
}
72+
73+
private class SafeSetEndpointIdentificationAlgorithm extends MethodAccess {
74+
SafeSetEndpointIdentificationAlgorithm() {
75+
this.getMethod().hasName("setEndpointIdentificationAlgorithm") and
76+
this.getMethod().getDeclaringType() instanceof SSLParameters and
77+
not this.getArgument(0) instanceof NullLiteral and
78+
not this.getArgument(0).(CompileTimeConstantExpr).getStringValue().length() = 0
79+
}
80+
}
81+
82+
/**
83+
* A call to the method `useSslProtocol` on an instance of `com.rabbitmq.client.ConnectionFactory`
84+
* that doesn't have `enableHostnameVerification` set.
85+
*/
86+
class RabbitMQEnableHostnameVerificationNotSet extends MethodAccess {
87+
RabbitMQEnableHostnameVerificationNotSet() {
88+
this.getMethod().hasName("useSslProtocol") and
89+
this.getMethod().getDeclaringType() instanceof RabbitMQConnectionFactory and
90+
exists(Variable v |
91+
v.getType() instanceof RabbitMQConnectionFactory and
92+
this.getQualifier() = v.getAnAccess() and
93+
not exists(MethodAccess ma |
94+
ma.getMethod().hasName("enableHostnameVerification") and
95+
ma.getQualifier() = v.getAnAccess()
96+
)
97+
)
98+
}
99+
}
100+
101+
private class RabbitMQConnectionFactory extends RefType {
102+
RabbitMQConnectionFactory() { this.hasQualifiedName("com.rabbitmq.client", "ConnectionFactory") }
103+
}

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

Lines changed: 14 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -12,158 +12,25 @@
1212
*/
1313

1414
import java
15-
import semmle.code.java.security.Encryption
15+
import semmle.code.java.dataflow.TaintTracking
16+
import semmle.code.java.security.UnsafeCertTrust
1617

17-
class SSLEngine extends RefType {
18-
SSLEngine() { this.hasQualifiedName("javax.net.ssl", "SSLEngine") }
19-
}
20-
21-
class Socket extends RefType {
22-
Socket() { this.hasQualifiedName("java.net", "Socket") }
23-
}
18+
class SslEndpointIdentificationFlowConfig extends TaintTracking::Configuration {
19+
SslEndpointIdentificationFlowConfig() { this = "SslEndpointIdentificationFlowConfig" }
2420

25-
class SocketFactory extends RefType {
26-
SocketFactory() { this.hasQualifiedName("javax.net", "SocketFactory") }
27-
}
21+
override predicate isSource(DataFlow::Node source) { source instanceof SslConnectionInit }
2822

29-
class SSLSocket extends RefType {
30-
SSLSocket() { this.hasQualifiedName("javax.net.ssl", "SSLSocket") }
31-
}
23+
override predicate isSink(DataFlow::Node sink) { sink instanceof SslConnectionCreation }
3224

33-
/**
34-
* has setEndpointIdentificationAlgorithm set correctly
35-
*/
36-
predicate setEndpointIdentificationAlgorithm(MethodAccess createSSL) {
37-
exists(
38-
Variable sslo, MethodAccess ma, Variable sslparams //setSSLParameters with valid setEndpointIdentificationAlgorithm set
39-
|
40-
createSSL = sslo.getAnAssignedValue() and
41-
ma.getQualifier() = sslo.getAnAccess() and
42-
ma.getMethod().hasName("setSSLParameters") and
43-
ma.getArgument(0) = sslparams.getAnAccess() and
44-
exists(MethodAccess setepa |
45-
setepa.getQualifier() = sslparams.getAnAccess() and
46-
setepa.getMethod().hasName("setEndpointIdentificationAlgorithm") and
47-
not setepa.getArgument(0) instanceof NullLiteral
48-
)
49-
)
50-
}
51-
52-
/**
53-
* has setEndpointIdentificationAlgorithm set correctly
54-
*/
55-
predicate hasEndpointIdentificationAlgorithm(Variable ssl) {
56-
exists(
57-
MethodAccess ma, Variable sslparams //setSSLParameters with valid setEndpointIdentificationAlgorithm set
58-
|
59-
ma.getQualifier() = ssl.getAnAccess() and
60-
ma.getMethod().hasName("setSSLParameters") and
61-
ma.getArgument(0) = sslparams.getAnAccess() and
62-
exists(MethodAccess setepa |
63-
setepa.getQualifier() = sslparams.getAnAccess() and
64-
setepa.getMethod().hasName("setEndpointIdentificationAlgorithm") and
65-
not setepa.getArgument(0) instanceof NullLiteral
66-
)
67-
)
68-
}
69-
70-
/**
71-
* Cast of Socket to SSLSocket
72-
*/
73-
predicate sslCast(MethodAccess createSSL) {
74-
exists(Variable ssl, CastExpr ce |
75-
ce.getExpr() = createSSL and
76-
ce.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl and
77-
ssl.getType() instanceof SSLSocket //With a type cast `SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443)`
78-
)
79-
}
80-
81-
/**
82-
* SSL object is created in a separate method call or in the same method
83-
*/
84-
predicate hasFlowPath(MethodAccess createSSL, Variable ssl) {
85-
(
86-
createSSL = ssl.getAnAssignedValue()
87-
or
88-
exists(CastExpr ce |
89-
ce.getExpr() = createSSL and
90-
ce.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl //With a type cast like SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443);
91-
)
92-
)
93-
or
94-
exists(MethodAccess tranm |
95-
createSSL.getEnclosingCallable() = tranm.getMethod() and
96-
tranm.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl and
97-
not setEndpointIdentificationAlgorithm(createSSL) //Check the scenario of invocation before used in the current method
98-
)
99-
}
100-
101-
/**
102-
* Not have the SSLParameter set
103-
*/
104-
predicate hasNoEndpointIdentificationSet(MethodAccess createSSL, Variable ssl) {
105-
//No setSSLParameters set
106-
hasFlowPath(createSSL, ssl) and
107-
not exists(MethodAccess ma |
108-
ma.getQualifier() = ssl.getAnAccess() and
109-
ma.getMethod().hasName("setSSLParameters")
110-
)
111-
or
112-
//No endpointIdentificationAlgorithm set with setSSLParameters
113-
hasFlowPath(createSSL, ssl) and
114-
not setEndpointIdentificationAlgorithm(createSSL)
115-
}
116-
117-
/**
118-
* The setEndpointIdentificationAlgorithm method of SSLParameters with the ssl engine or socket
119-
*/
120-
class SSLEndpointIdentificationNotSet extends MethodAccess {
121-
SSLEndpointIdentificationNotSet() {
122-
(
123-
this.getMethod().hasName("createSSLEngine") and
124-
this.getMethod().getDeclaringType() instanceof SSLContext //createEngine method of SSLContext
125-
or
126-
this.getMethod().hasName("createSocket") and
127-
this.getMethod().getDeclaringType() instanceof SocketFactory and
128-
this.getMethod().getReturnType() instanceof Socket and
129-
sslCast(this) //createSocket method of SocketFactory
130-
) and
131-
exists(Variable ssl |
132-
hasNoEndpointIdentificationSet(this, ssl) and //Not set in itself
133-
not exists(VariableAssign ar, Variable newSsl |
134-
ar.getSource() = this.getCaller().getAReference() and
135-
ar.getDestVar() = newSsl and
136-
hasEndpointIdentificationAlgorithm(newSsl) //Not set in its caller either
137-
)
138-
) and
139-
not exists(MethodAccess ma | ma.getMethod() instanceof HostnameVerifierVerify) //Reduce false positives since this method access set default hostname verifier
140-
}
141-
}
142-
143-
class RabbitMQConnectionFactory extends RefType {
144-
RabbitMQConnectionFactory() { this.hasQualifiedName("com.rabbitmq.client", "ConnectionFactory") }
145-
}
146-
147-
/**
148-
* The com.rabbitmq.client.ConnectionFactory useSslProtocol method access without enableHostnameVerification
149-
*/
150-
class RabbitMQEnableHostnameVerificationNotSet extends MethodAccess {
151-
RabbitMQEnableHostnameVerificationNotSet() {
152-
this.getMethod().hasName("useSslProtocol") and
153-
this.getMethod().getDeclaringType() instanceof RabbitMQConnectionFactory and
154-
exists(Variable v |
155-
v.getType() instanceof RabbitMQConnectionFactory and
156-
this.getQualifier() = v.getAnAccess() and
157-
not exists(MethodAccess ma |
158-
ma.getMethod().hasName("enableHostnameVerification") and
159-
ma.getQualifier() = v.getAnAccess()
160-
)
161-
)
25+
override predicate isSanitizer(DataFlow::Node sanitizer) {
26+
sanitizer.asExpr() instanceof SslConnectionWithSafeSslParameters
16227
}
16328
}
16429

165-
from MethodAccess aa
30+
from Expr unsafeConfig
16631
where
167-
aa instanceof SSLEndpointIdentificationNotSet or
168-
aa instanceof RabbitMQEnableHostnameVerificationNotSet
169-
select aa, "Unsafe configuration of trusted certificates"
32+
unsafeConfig instanceof RabbitMQEnableHostnameVerificationNotSet or
33+
exists(SslEndpointIdentificationFlowConfig config |
34+
config.hasFlowTo(DataFlow::exprNode(unsafeConfig))
35+
)
36+
select unsafeConfig, "Unsafe configuration of trusted certificates"

0 commit comments

Comments
 (0)