hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 11:15:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

Java: Remove overlapping code

Browse Source
This commit is contained in:
intrigus
2020-12-05 00:32:44 +01:00
parent 3da1cb0879
commit 70b0703952
4 changed files with 6 additions and 101 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.java
View File

@@ -1,30 +1,4 @@
public static void main(String[] args) {
{
HostnameVerifier verifier = new HostnameVerifier() {
@Override
public boolean verify(String hostname, SSLSession session) {
try { //GOOD: verify the certificate
Certificate[] certs = session.getPeerCertificates();
X509Certificate x509 = (X509Certificate) certs[0];
check(new String[]{host}, x509);
return true;
} catch (SSLException e) {
return false;
}
}
};
HttpsURLConnection.setDefaultHostnameVerifier(verifier);
}
{
HostnameVerifier verifier = new HostnameVerifier() {
@Override
public boolean verify(String hostname, SSLSession session) {
return true; // BAD: accept even if the hostname doesn't match
}
};
HttpsURLConnection.setDefaultHostnameVerifier(verifier);
}
{
X509TrustManager trustAllCertManager = new X509TrustManager() {

8
java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.qhelp
View File

@@ -4,10 +4,10 @@
<qhelp>
<overview>
<p>Java offers two mechanisms for SSL authentication - trust manager and hostname verifier. Trust manager validates the peer's certificate chain while hostname verification establishes that the hostname in the URL matches the hostname in the server's identification.</p>
<p>Java offers two mechanisms for SSL authentication - trust manager and hostname verifier (not checked by this query). Trust manager validates the peer's certificate chain while hostname verification establishes that the hostname in the URL matches the hostname in the server's identification.</p>
<p>And when SSLSocket or SSLEngine is created without a valid parameter of setEndpointIdentificationAlgorithm, hostname verification is disabled by default.</p>
<p>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.</p>
<p>This query checks whether trust manager is set to trust all certificates, the hostname verifier is turned off, or setEndpointIdentificationAlgorithm is missing. The query also covers a special implementation com.rabbitmq.client.ConnectionFactory.</p>
<p>Unsafe implementation of the interface X509TrustManager 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.</p>
<p>This query checks whether trust manager is set to trust all certificates or setEndpointIdentificationAlgorithm is missing. The query also covers a special implementation com.rabbitmq.client.ConnectionFactory.</p>
</overview>
<recommendation>
@@ -15,7 +15,7 @@
</recommendation>
<example>
<p>The following two examples show two ways of configuring X509 trust cert manager and hostname verifier. In the 'BAD' case,
<p>The following two examples show two ways of configuring X509 trust cert manager. In the 'BAD' case,
no validation is performed thus any certificate is trusted. In the 'GOOD' case, the proper validation is performed.</p>
<sample src="UnsafeCertTrust.java" />
</example>

41
java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql
View File

@@ -1,6 +1,6 @@
/**
* @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.
* @name Unsafe certificate trust
* @description Unsafe implementation of the interface X509TrustManager 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
* @tags security
@@ -53,42 +53,6 @@ class X509TrustAllManagerInit extends MethodAccess {
}
}
/**
* HostnameVerifier class that allows a certificate whose CN (Common Name) does not match the host name in the URL
*/
class TrustAllHostnameVerifier extends RefType {
TrustAllHostnameVerifier() {
this.getASupertype*() instanceof HostnameVerifier and
exists(Method m, ReturnStmt rt |
m.getDeclaringType() = this and
m.hasName("verify") and
rt.getEnclosingCallable() = m and
rt.getResult().(BooleanLiteral).getBooleanValue() = true
)
}
}
/**
* The setDefaultHostnameVerifier method of HttpsURLConnection with the trust all configuration
*/
class TrustAllHostnameVerify extends MethodAccess {
TrustAllHostnameVerify() {
this.getMethod().hasName("setDefaultHostnameVerifier") and
this.getMethod().getDeclaringType() instanceof HttpsURLConnection and //httpsURLConnection.setDefaultHostnameVerifier method
(
exists(NestedClass nc |
nc.getASupertype*() instanceof TrustAllHostnameVerifier and
this.getArgument(0).getType() = nc //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() {...});
)
or
exists(Variable v |
this.getArgument(0).(VarAccess).getVariable() = v and
v.getInitializer().getType() instanceof TrustAllHostnameVerifier //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(verifier);
)
)
}
}
class SSLEngine extends RefType {
SSLEngine() { this.hasQualifiedName("javax.net.ssl", "SSLEngine") }
}
@@ -239,7 +203,6 @@ class RabbitMQEnableHostnameVerificationNotSet extends MethodAccess {
from MethodAccess aa
where
aa instanceof TrustAllHostnameVerify or
aa instanceof X509TrustAllManagerInit or
aa instanceof SSLEndpointIdentificationNotSet or
aa instanceof RabbitMQEnableHostnameVerificationNotSet

32
java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrustTest.java
View File

@@ -48,31 +48,6 @@ public class UnsafeCertTrustTest {
}
}
/**
* Test the implementation of trusting all hostnames as an anonymous class
*/
public void testTrustAllHostnameOfAnonymousClass() {
HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() {
@Override
public boolean verify(String hostname, SSLSession session) {
return true; // Noncompliant
}
});
}
/**
* Test the implementation of trusting all hostnames as a variable
*/
public void testTrustAllHostnameOfVariable() {
HostnameVerifier verifier = new HostnameVerifier() {
@Override
public boolean verify(String hostname, SSLSession session) {
return true; // Noncompliant
}
};
HttpsURLConnection.setDefaultHostnameVerifier(verifier);
}
private static final X509TrustManager TRUST_ALL_CERTIFICATES = new X509TrustManager() {
@Override
public void checkClientTrusted(final X509Certificate[] chain, final String authType)
@@ -109,13 +84,6 @@ public class UnsafeCertTrustTest {
}
};
public static final HostnameVerifier ALLOW_ALL_HOSTNAME_VERIFIER = new HostnameVerifier() {
@Override
public boolean verify(String hostname, SSLSession session) {
return true; // Noncompliant
}
};
/**
* Test the endpoint identification of SSL engine is set to null
*/