Skip to content

Java: CWE-273 Unsafe certificate trust #3550

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aschackmull merged 5 commits into from
Jun 29, 2020
Merged

Java: CWE-273 Unsafe certificate trust #3550

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6d1ba3f
Java: CWE-273 Unsafe certificate trust
luchua-bc May 24, 2020
104f1c3
Add validation query for SSL Engine/Socket and com.rabbitmq.client.Co…
luchua-bc May 28, 2020
deabfe6
Adjust id tag and fix ending line error
luchua-bc Jun 23, 2020
f8c4947
Fix ending line error
luchua-bc Jun 23, 2020
0779aab
Clean up the QL code
luchua-bc Jun 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Clean up the QL code
  • Loading branch information
@luchua-bc
luchua-bc committed Jun 24, 2020
commit 0779aab28fe1baa0db8a70862e932a7a05e2a670
46 changes: 32 additions & 14 deletions java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* @name Unsafe implementation of trusting any certificate or missing hostname verification in SSL configuration
* @name Unsafe certificate trust and improper hostname verification
* @description Unsafe implementation of the interface X509TrustManager, HostnameVerifier, and SSLSocket/SSLEngine ignores all SSL certificate validation errors when establishing an HTTPS connection, thereby making the app vulnerable to man-in-the-middle attacks.
* @kind problem
* @id java/unsafe-cert-trust
Expand Down Expand Up @@ -39,13 +39,14 @@ class X509TrustAllManagerInit extends MethodAccess {
this.getMethod().getDeclaringType() instanceof SSLContext and //init method of SSLContext
(
exists(ArrayInit ai |
this.getArgument(1).(ArrayCreationExpr).getInit() = ai and
ai.getInit(0).(VarAccess).getVariable().getInitializer().getType().(Class).getASupertype*()
instanceof X509TrustAllManager //Scenario of context.init(null, new TrustManager[] { TRUST_ALL_CERTIFICATES }, null);
)
Comment thread
luchua-bc marked this conversation as resolved.
or
exists(Variable v, ArrayInit ai |
this.getArgument(1).(VarAccess).getVariable() = v and
ai.getParent() = v.getAnAccess().getVariable().getAnAssignedValue() and
ai.getParent() = v.getAnAssignedValue() and
ai.getInit(0).getType().(Class).getASupertype*() instanceof X509TrustAllManager //Scenario of context.init(null, serverTMs, null);
)
)
Expand Down Expand Up @@ -81,7 +82,7 @@ class TrustAllHostnameVerify extends MethodAccess {
)
or
exists(Variable v |
this.getArgument(0).(VarAccess).getVariable() = v.getAnAccess().getVariable() and
this.getArgument(0).(VarAccess).getVariable() = v and
v.getInitializer().getType() instanceof TrustAllHostnameVerifier //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(verifier);
)
)
Expand All @@ -96,6 +97,10 @@ class Socket extends RefType {
Socket() { this.hasQualifiedName("java.net", "Socket") }
}

class SocketFactory extends RefType {
SocketFactory() { this.hasQualifiedName("javax.net", "SocketFactory") }
}

class SSLSocket extends RefType {
SSLSocket() { this.hasQualifiedName("javax.net.ssl", "SSLSocket") }
}
Expand All @@ -110,9 +115,9 @@ predicate setEndpointIdentificationAlgorithm(MethodAccess createSSL) {
createSSL = sslo.getAnAssignedValue() and
ma.getQualifier() = sslo.getAnAccess() and
ma.getMethod().hasName("setSSLParameters") and
ma.getArgument(0).(VarAccess) = sslparams.getAnAccess() and
ma.getArgument(0) = sslparams.getAnAccess() and
exists(MethodAccess setepa |
setepa.getQualifier().(VarAccess) = sslparams.getAnAccess() and
setepa.getQualifier() = sslparams.getAnAccess() and
setepa.getMethod().hasName("setEndpointIdentificationAlgorithm") and
not setepa.getArgument(0) instanceof NullLiteral
)
Expand All @@ -128,15 +133,26 @@ predicate hasEndpointIdentificationAlgorithm(Variable ssl) {
|
ma.getQualifier() = ssl.getAnAccess() and
ma.getMethod().hasName("setSSLParameters") and
ma.getArgument(0).(VarAccess) = sslparams.getAnAccess() and
ma.getArgument(0) = sslparams.getAnAccess() and
exists(MethodAccess setepa |
setepa.getQualifier().(VarAccess) = sslparams.getAnAccess() and
setepa.getQualifier() = sslparams.getAnAccess() and
setepa.getMethod().hasName("setEndpointIdentificationAlgorithm") and
not setepa.getArgument(0) instanceof NullLiteral
)
)
}

/**
* Cast of Socket to SSLSocket
*/
predicate sslCast(MethodAccess createSSL) {
exists(Variable ssl, CastExpr ce |
ce.getExpr() = createSSL and
ce.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl and
ssl.getType() instanceof SSLSocket //With a type cast `SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443)`
)
}

/**
* SSL object is created in a separate method call or in the same method
*/
Expand All @@ -145,13 +161,13 @@ predicate hasFlowPath(MethodAccess createSSL, Variable ssl) {
createSSL = ssl.getAnAssignedValue()
or
exists(CastExpr ce |
ce.getExpr().(MethodAccess) = createSSL and
ce.getExpr() = createSSL and
ce.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl //With a type cast like SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443);
)
)
or
exists(MethodAccess tranm |
createSSL.getEnclosingCallable().(Method) = tranm.getMethod() and
createSSL.getEnclosingCallable() = tranm.getMethod() and
tranm.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl and
not setEndpointIdentificationAlgorithm(createSSL) //Check the scenario of invocation before used in the current method
)
Expand Down Expand Up @@ -183,7 +199,9 @@ class SSLEndpointIdentificationNotSet extends MethodAccess {
this.getMethod().getDeclaringType() instanceof SSLContext //createEngine method of SSLContext
or
this.getMethod().hasName("createSocket") and
this.getMethod().getReturnType() instanceof Socket //createSocket method of SSLSocketFactory
this.getMethod().getDeclaringType() instanceof SocketFactory and
this.getMethod().getReturnType() instanceof Socket and
sslCast(this) //createSocket method of SocketFactory
) and
exists(Variable ssl |
hasNoEndpointIdentificationSet(this, ssl) and //Not set in itself
Expand All @@ -208,12 +226,12 @@ class RabbitMQEnableHostnameVerificationNotSet extends MethodAccess {
RabbitMQEnableHostnameVerificationNotSet() {
this.getMethod().hasName("useSslProtocol") and
this.getMethod().getDeclaringType() instanceof RabbitMQConnectionFactory and
exists(VarAccess va |
va.getVariable().getType() instanceof RabbitMQConnectionFactory and
this.getQualifier() = va.getVariable().getAnAccess() and
exists(Variable v |
v.getType() instanceof RabbitMQConnectionFactory and
this.getQualifier() = v.getAnAccess() and
not exists(MethodAccess ma |
ma.getMethod().hasName("enableHostnameVerification") and
ma.getQualifier() = va.getVariable().getAnAccess()
ma.getQualifier() = v.getAnAccess()
)
)
}
Expand Down
14 changes: 7 additions & 7 deletions java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrust.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
| UnsafeCertTrustTest.java:26:4:26:74 | init(...) | Unsafe configuration of trusted certificates |
| UnsafeCertTrustTest.java:41:4:41:38 | init(...) | Unsafe configuration of trusted certificates |
| UnsafeCertTrustTest.java:54:3:59:4 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates |
| UnsafeCertTrustTest.java:72:3:72:57 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates |
| UnsafeCertTrustTest.java:123:25:123:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates |
| UnsafeCertTrustTest.java:134:25:134:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates |
| UnsafeCertTrustTest.java:143:34:143:83 | createSocket(...) | Unsafe configuration of trusted certificates |
| UnsafeCertTrustTest.java:27:4:27:74 | init(...) | Unsafe configuration of trusted certificates |
| UnsafeCertTrustTest.java:42:4:42:38 | init(...) | Unsafe configuration of trusted certificates |
| UnsafeCertTrustTest.java:55:3:60:4 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates |
| UnsafeCertTrustTest.java:73:3:73:57 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates |
| UnsafeCertTrustTest.java:124:25:124:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates |
| UnsafeCertTrustTest.java:135:25:135:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates |
| UnsafeCertTrustTest.java:144:34:144:83 | createSocket(...) | Unsafe configuration of trusted certificates |
19 changes: 14 additions & 5 deletions java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrustTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import javax.net.ssl.X509TrustManager;

import java.net.Socket;
import javax.net.SocketFactory;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;

Expand Down Expand Up @@ -126,7 +127,7 @@ public void testSSLEngineEndpointIdSetNull() {
sslEngine.setSSLParameters(sslParameters);
}

/**
/**
* Test the endpoint identification of SSL engine is not set
*/
public void testSSLEngineEndpointIdNotSet() {
Expand All @@ -143,11 +144,19 @@ public void testSSLSocketEndpointIdNotSet() {
SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443);
}

/**
* Test the endpoint identification of regular socket is not set
*/
public void testSocketEndpointIdNotSet() {
SocketFactory socketFactory = SocketFactory.getDefault();
Socket socket = socketFactory.createSocket("www.example.com", 80);
}

// /**
// * Test the enableHostnameVerification of RabbitMQConnectionFactory is not set
// */
// * Test the enableHostnameVerification of RabbitMQConnectionFactory is not set
// */
// public void testEnableHostnameVerificationOfRabbitMQFactoryNotSet() {
// ConnectionFactory connectionFactory = new ConnectionFactory();
// connectionFactory.useSslProtocol();
// ConnectionFactory connectionFactory = new ConnectionFactory();
// connectionFactory.useSslProtocol();
// }
}