Java: Add JXBrowser disabled certificate query.

This commit is contained in:
intrigus
2021-01-12 14:48:22 +01:00
parent 4bc287e89b
commit 9b3070ab7c
3 changed files with 127 additions and 0 deletions

View File

@@ -0,0 +1,23 @@
public static void main(String[] args) {
{
Browser browser = new Browser();
browser.loadURL("https://example.com");
// no further calls
// BAD: The browser ignores any certificate error by default!
}
{
Browser browser = new Browser();
browser.setLoadHandler(new LoadHandler() {
public boolean onLoad(LoadParams params) {
return true;
}
public boolean onCertificateError(CertificateErrorParams params){
return true; // GOOD: This means that loading will be cancelled on certificate errors
}
}); // GOOD: A secure `LoadHandler` is used.
browser.loadURL("https://example.com");
}
}

View File

@@ -0,0 +1,31 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>JXBrowser is a Java library that allows to embed the Chromium browser inside Java applications.
The version 6.x.x by default ignores any HTTPS certificate errors thereby allowing man-in-the-middle attacks.
</p>
</overview>
<recommendation>
<p>Do either of these:
<li>Update to version 7.x.x as it now correctly rejects certificate errors by default.</li>
<li>Add a custom implementation of the <code>LoadHandler</code> interface whose <code>onCertificateError</code> method always returns <b>true</b> indicating that loading should be cancelled.
Then use the <code>setLoadHandler</code> method with your custom <code>LoadHandler</code> on every <code>Browser</code> you use.</li>
</p>
</recommendation>
<example>
<p>The following two examples show two ways of using a <code>Browser</code>. In the 'BAD' case,
all certificate errors are ignored. In the 'GOOD' case, certificate errors are rejected.</p>
<sample src="JXBrowserWithoutCertValidation.java" />
</example>
<references>
<li>Teamdev:
<a href="https://www.teamdev.com/downloads/jxbrowser/javadoc/com/teamdev/jxbrowser/chromium/LoadHandler.html#onCertificateError-com.teamdev.jxbrowser.chromium.CertificateErrorParams-">
Javadoc for the LoadHandler#onCertificateError method</a>.</li>
</references>
</qhelp>

View File

@@ -0,0 +1,73 @@
/**
* @name JXBrowser with disabled certificate validation
* @description Insecure configuration of JXBrowser disables certificate validation making the app vulnerable to man-in-the-middle attacks.
* @kind problem
* @id java/jxbrowser/disabled-certificate-validation
* @tags security
* external/cwe-295
*/
import java
import semmle.code.java.security.Encryption
import semmle.code.java.dataflow.TaintTracking
/*
* This query is version specific to JXBrowser 6.x.x. The version is indirectly detected.
* In version 6.x.x the `Browser` class is in a different package compared to version 7.x.x.
*/
/** The `com.teamdev.jxbrowser.chromium.Browser` class. */
private class JXBrowser extends RefType {
JXBrowser() { this.hasQualifiedName("com.teamdev.jxbrowser.chromium", "Browser") }
}
/** The `setLoadHandler` method on the `com.teamdev.jxbrowser.chromium.Browser` class. */
private class JXBrowserSetLoadHandler extends Method {
JXBrowserSetLoadHandler() {
this.hasName("setLoadHandler") and this.getDeclaringType() instanceof JXBrowser
}
}
/** The `com.teamdev.jxbrowser.chromium.LoadHandler` interface. */
private class JXBrowserLoadHandler extends RefType {
JXBrowserLoadHandler() { this.hasQualifiedName("com.teamdev.jxbrowser.chromium", "LoadHandler") }
}
private predicate isOnCertificateErrorMethodSafe(Method m) {
forex(ReturnStmt rs | rs.getEnclosingCallable() = m |
rs.getResult().(CompileTimeConstantExpr).getBooleanValue() = true
)
}
/** A class that securely implements the `com.teamdev.jxbrowser.chromium.LoadHandler` interface. */
private class JXBrowserSafeLoadHandler extends RefType {
JXBrowserSafeLoadHandler() {
this.getASupertype() instanceof JXBrowserLoadHandler and
exists(Method m | m.hasName("onCertificateError") and m.getDeclaringType() = this |
isOnCertificateErrorMethodSafe(m)
)
}
}
private class JXBrowserTaintTracking extends TaintTracking::Configuration {
JXBrowserTaintTracking() { this = "JXBrowserTaintTracking" }
override predicate isSource(DataFlow::Node src) {
exists(ClassInstanceExpr newJXBrowser | newJXBrowser.getConstructedType() instanceof JXBrowser |
newJXBrowser = src.asExpr()
)
}
override predicate isSink(DataFlow::Node sink) {
exists(MethodAccess ma | ma.getMethod() instanceof JXBrowserSetLoadHandler |
ma.getArgument(0).getType() instanceof JXBrowserSafeLoadHandler and
ma.getQualifier() = sink.asExpr()
)
}
}
from JXBrowserTaintTracking cfg, DataFlow::Node src
where
cfg.isSource(src) and
not cfg.hasFlow(src, _)
select src, "This JXBrowser instance allows man-in-the-middle attacks."