mirror of
https://github.com/github/codeql.git
synced 2025-12-24 04:36:35 +01:00
Java: Add insecure trust manager query.
This commit is contained in:
intrigus
50
java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java
Normal file
50
java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java
Normal file
@@ -0,0 +1,50 @@
|
||||
public static void main(String[] args) throws Exception {
|
||||
{
|
||||
class InsecureTrustManager implements X509TrustManager {
|
||||
@Override
|
||||
public X509Certificate[] getAcceptedIssuers() {
|
||||
return null;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
|
||||
// BAD: Does not verify the certificate chain, allowing any certificate.
|
||||
}
|
||||
|
||||
@Override
|
||||
public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
|
||||
|
||||
}
|
||||
}
|
||||
SSLContext context = SSLContext.getInstance("TLS");
|
||||
TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() };
|
||||
context.init(null, trustManager, null);
|
||||
}
|
||||
{
|
||||
SSLContext context = SSLContext.getInstance("TLS");
|
||||
File certificateFile = new File("path/to/self-signed-certificate");
|
||||
// Create a `KeyStore` with default type
|
||||
KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
|
||||
// This causes `keyStore` to be empty
|
||||
keyStore.load(null, null);
|
||||
X509Certificate generatedCertificate;
|
||||
try (InputStream cert = new FileInputStream(certificateFile)) {
|
||||
generatedCertificate = (X509Certificate) CertificateFactory.getInstance("X509")
|
||||
.generateCertificate(cert);
|
||||
}
|
||||
// Add the self-signed certificate to the key store
|
||||
keyStore.setCertificateEntry(certificateFile.getName(), generatedCertificate);
|
||||
// Get default `TrustManagerFactory`
|
||||
TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
|
||||
// Use it with our modified key store that trusts our self-signed certificate
|
||||
tmf.init(keyStore);
|
||||
TrustManager[] trustManagers = tmf.getTrustManagers();
|
||||
context.init(null, trustManagers, null); // GOOD, we are not using a custom `TrustManager` but instead have
|
||||
// added the self-signed certificate we want to trust to the key
|
||||
// store. Note, the `trustManagers` will **only** trust this one
|
||||
// certificate.
|
||||
URL url = new URL("https://self-signed.badssl.com/");
|
||||
HttpsURLConnection conn = (HttpsURLConnection) url.openConnection();
|
||||
conn.setSSLSocketFactory(context.getSocketFactory());
|
||||
}
|
||||
}
|
||||
43
java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp
Normal file
43
java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp
Normal file
@@ -0,0 +1,43 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>
|
||||
If the <code>checkServerTrusted</code> method of a <code>TrustManager</code> never throws a <code>CertificateException</code> it trusts every certificate.
|
||||
This allows an attacker to perform a Man-in-the-middle attack against the application therefore breaking any security Transport Layer Security (TLS) gives.
|
||||
|
||||
An attack would look like this:
|
||||
1. The program connects to <code>https://example.com</code>.
|
||||
2. The attacker intercepts this connection and presents a valid, self-signed certificate for <code>https://example.com</code>.
|
||||
3. Java calls the <code>checkServerTrusted</code> method to check whether it should trust the certificate.
|
||||
4. The <code>checkServerTrusted</code> method of your <code>TrustManager</code> does not throw a <code>CertificateException</code>.
|
||||
5. Java proceeds with the connection since your <code>TrustManager</code> implicitly trusted it by not throwing an exception.
|
||||
6. The attacker can now read the data your program sends to <code>https://example.com</code> and/or alter its replies while the program thinks the connection is secure.
|
||||
</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
Do not use a custom <code>TrustManager</code> that trusts any certificate.
|
||||
If you have to use a self-signed certificate, don't trust every certificate, but instead only trust this specific certificate.
|
||||
See below for an example of how to do this.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>
|
||||
In the first (bad) example, the <code>TrustManager</code> never throws a <code>CertificateException</code> thereby trusting any certificate.
|
||||
This allows an attacker to perform a man-in-the-middle attack.
|
||||
In the second (good) example, no custom <code>TrustManager</code> is used. Instead, the self-signed certificate that should be trusted
|
||||
is explicitly trusted by loading it into a <code>KeyStore</code>.
|
||||
</p>
|
||||
<sample src="InsecureTrustManager.java" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li><a href="https://developer.android.com/training/articles/security-ssl">Android Security Guide for TLS/HTTPS</a>.</li>
|
||||
<li>OWASP: <a href="https://cwe.mitre.org/data/definitions/295.html">CWE-295</a>.</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
180
java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql
Normal file
180
java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql
Normal file
@@ -0,0 +1,180 @@
|
||||
/**
|
||||
* @name Everything trusting `TrustManager`
|
||||
* @description Trusting all certificates allows an attacker to perform a machine-in-the-middle attack.
|
||||
* @kind path-problem
|
||||
* @problem.severity error
|
||||
* @precision high
|
||||
* @id java/insecure-trustmanager
|
||||
* @tags security
|
||||
* external/cwe/cwe-295
|
||||
*/
|
||||
|
||||
import java
|
||||
import semmle.code.java.controlflow.Guards
|
||||
import semmle.code.java.dataflow.DataFlow
|
||||
import semmle.code.java.dataflow.FlowSources
|
||||
import semmle.code.java.dataflow.TaintTracking2
|
||||
import semmle.code.java.security.Encryption
|
||||
import DataFlow::PathGraph
|
||||
|
||||
/**
|
||||
* Models an insecure `X509TrustManager`.
|
||||
* An `X509TrustManager` is considered insecure if it never throws a `CertificatException` thereby accepting any certificate as valid.
|
||||
*/
|
||||
class InsecureX509TrustManager extends RefType {
|
||||
InsecureX509TrustManager() {
|
||||
getASupertype*() instanceof X509TrustManager and
|
||||
exists(Method m |
|
||||
m.getDeclaringType() = this and
|
||||
m.hasName("checkServerTrusted") and
|
||||
not mayThrowCertificateException(m)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/** The `java.security.cert.CertificateException` class. */
|
||||
private class CertificatException extends RefType {
|
||||
CertificatException() { hasQualifiedName("java.security.cert", "CertificateException") }
|
||||
}
|
||||
|
||||
/**
|
||||
*Holds if:
|
||||
* - `m` may `throw` an `CertificatException`
|
||||
* - `m` calls another method that may throw
|
||||
* - `m` calls a method that declares to throw an `CertificatExceptio`, but for which no source is available
|
||||
*/
|
||||
private predicate mayThrowCertificateException(Method m) {
|
||||
exists(Stmt stmt | m.getBody().getAChild*() = stmt |
|
||||
stmt.(ThrowStmt).getThrownExceptionType().getASupertype*() instanceof CertificatException
|
||||
)
|
||||
or
|
||||
exists(Method otherMethod | m.polyCalls(otherMethod) |
|
||||
mayThrowCertificateException(otherMethod)
|
||||
or
|
||||
not otherMethod.fromSource() and
|
||||
otherMethod.getAnException().getType().getASupertype*() instanceof CertificatException
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* A configuration to model the flow of a `InsecureX509TrustManager` to an `SSLContext.init` call.
|
||||
*/
|
||||
class InsecureTrustManagerConfiguration extends TaintTracking::Configuration {
|
||||
InsecureTrustManagerConfiguration() { this = "InsecureTrustManagerConfiguration" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) {
|
||||
source.asExpr().(ClassInstanceExpr).getConstructedType() instanceof InsecureX509TrustManager
|
||||
}
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
exists(MethodAccess ma, Method m |
|
||||
m.hasName("init") and
|
||||
m.getDeclaringType() instanceof SSLContext and
|
||||
ma.getMethod() = m
|
||||
|
|
||||
ma.getArgument(1) = sink.asExpr()
|
||||
)
|
||||
}
|
||||
|
||||
override predicate isSanitizer(DataFlow::Node barrier) {
|
||||
// ignore nodes that are in functions that intentionally trust all certificates
|
||||
barrier
|
||||
.getEnclosingCallable()
|
||||
.getName()
|
||||
/*
|
||||
* Regex: (_)* :
|
||||
* some methods have underscores.
|
||||
* Regex: (no|ignore|disable)(strictssl|ssl|verify|verification)
|
||||
* noStrictSSL ignoreSsl
|
||||
* Regex: (set)?(accept|trust|ignore|allow)(all|every|any|selfsigned)
|
||||
* acceptAll trustAll ignoreAll setTrustAnyHttps
|
||||
* Regex: (use|do|enable)insecure
|
||||
* useInsecureSSL
|
||||
* Regex: (set|do|use)?no.*(check|validation|verify|verification)
|
||||
* setNoCertificateCheck
|
||||
* Regex: disable
|
||||
* disableChecks
|
||||
*/
|
||||
|
||||
.regexpMatch("^(?i)(_)*((no|ignore|disable)(strictssl|ssl|verify|verification)" +
|
||||
"|(set)?(accept|trust|ignore|allow)(all|every|any|selfsigned)" +
|
||||
"|(use|do|enable)insecure|(set|do|use)?no.*(check|validation|verify|verification)|disable).*$")
|
||||
}
|
||||
}
|
||||
|
||||
bindingset[result]
|
||||
private string getAFlagName() {
|
||||
result
|
||||
.regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*")
|
||||
}
|
||||
|
||||
/**
|
||||
* A flag has to either be of type `String`, `boolean` or `Boolean`.
|
||||
*/
|
||||
private class FlagType extends Type {
|
||||
FlagType() {
|
||||
this instanceof TypeString
|
||||
or
|
||||
this instanceof BooleanType
|
||||
}
|
||||
}
|
||||
|
||||
private predicate isEqualsIgnoreCaseMethodAccess(MethodAccess ma) {
|
||||
ma.getMethod().hasName("equalsIgnoreCase") and
|
||||
ma.getMethod().getDeclaringType() instanceof TypeString
|
||||
}
|
||||
|
||||
/** Holds if `source` should is considered a flag. */
|
||||
private predicate isFlag(DataFlow::Node source) {
|
||||
exists(VarAccess v | v.getVariable().getName() = getAFlagName() |
|
||||
source.asExpr() = v and v.getType() instanceof FlagType
|
||||
)
|
||||
or
|
||||
exists(StringLiteral s | s.getRepresentedString() = getAFlagName() | source.asExpr() = s)
|
||||
or
|
||||
exists(MethodAccess ma | ma.getMethod().getName() = getAFlagName() |
|
||||
source.asExpr() = ma and
|
||||
ma.getType() instanceof FlagType and
|
||||
not isEqualsIgnoreCaseMethodAccess(ma)
|
||||
)
|
||||
}
|
||||
|
||||
/** Holds if there is flow from `node1` to `node2` either due to local flow or due to custom flow steps. */
|
||||
private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
|
||||
DataFlow::localFlowStep(node1, node2)
|
||||
or
|
||||
exists(MethodAccess ma | ma.getMethod() = any(EnvReadMethod m) |
|
||||
ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr()
|
||||
)
|
||||
or
|
||||
exists(MethodAccess ma |
|
||||
ma.getMethod().hasName("parseBoolean") and
|
||||
ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Boolean")
|
||||
|
|
||||
ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr()
|
||||
)
|
||||
}
|
||||
|
||||
/** Gets a guard that depends on a flag. */
|
||||
private Guard getAGuard() {
|
||||
exists(DataFlow::Node source, DataFlow::Node sink |
|
||||
isFlag(source) and
|
||||
flagFlowStep*(source, sink) and
|
||||
sink.asExpr() = result
|
||||
)
|
||||
}
|
||||
|
||||
/** Holds if `node` is guarded by a flag that suggests an intentionally insecure feature. */
|
||||
private predicate isNodeGuardedByFlag(DataFlow::Node node) {
|
||||
exists(Guard g | g.controls(node.asExpr().getBasicBlock(), _) | g = getAGuard())
|
||||
}
|
||||
|
||||
from
|
||||
DataFlow::PathNode source, DataFlow::PathNode sink, InsecureTrustManagerConfiguration cfg,
|
||||
RefType trustManager
|
||||
where
|
||||
cfg.hasFlowPath(source, sink) and
|
||||
not isNodeGuardedByFlag(sink.getNode()) and
|
||||
trustManager = source.getNode().asExpr().(ClassInstanceExpr).getConstructedType()
|
||||
select sink, source, sink, "$@ that is defined $@ and trusts any certificate, is used here.",
|
||||
source, "This trustmanager", trustManager, "here"
|
||||
Reference in New Issue
Block a user