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

Merge pull request #4945 from intrigus-lgtm/java/insecure-jxbrowser

Browse Source 
Java: Insecure JXBrowser
This commit is contained in:
Anders Schack-Mulligen
2021-02-10 15:48:17 +01:00
committed by GitHub
parent 18225fa254 5c82ff83de
commit b74911204a
20 changed files with 279 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
java/ql/src/experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.java Normal file
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");
}
}

32
java/ql/src/experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.qhelp Normal file
View File

@@ -0,0 +1,32 @@
<!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.
Versions smaller than 6.24 by default ignore any HTTPS certificate errors thereby allowing man-in-the-middle attacks.
</p>
</overview>
<recommendation>
<p>Do either of these:</p>
<ul>
<li>Update to version 6.24 or 7.x.x as these correctly reject 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>
</ul>
</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://jxbrowser.support.teamdev.com/support/discussions/topics/9000051708">
Changelog of JxBrowser 6.24</a>.</li>
</references>
</qhelp>

86
java/ql/src/experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.ql Normal file
View File

@@ -0,0 +1,86 @@
/**
* @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/cwe-295
*/
import java
import semmle.code.java.security.Encryption
import semmle.code.java.dataflow.DataFlow
/*
* This query is version specific to JxBrowser < 6.24. The version is indirectly detected.
* In version 6.x.x the `Browser` class is in a different package compared to version 7.x.x.
*/
/**
* Holds if a safe JxBrowser 6.x.x version is used, such as version 6.24.
* This is detected by the the presence of the `addBoundsListener` in the `Browser` class.
*/
private predicate isSafeJxBrowserVersion() {
exists(Method m | m.getDeclaringType() instanceof JxBrowser | m.hasName("addBoundsListener"))
}
/** 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)
)
}
}
/**
* Models flow from the source `new Browser()` to a sink `browser.setLoadHandler(loadHandler)` where `loadHandler`
* has been determined to be safe.
*/
private class JxBrowserFlowConfiguration extends DataFlow::Configuration {
JxBrowserFlowConfiguration() { this = "JxBrowserFlowConfiguration" }
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 JxBrowserFlowConfiguration cfg, DataFlow::Node src
where
cfg.isSource(src) and
not cfg.hasFlow(src, _) and
not isSafeJxBrowserVersion()
select src, "This JxBrowser instance may not check HTTPS certificates."

1
java/ql/test/experimental/query-tests/security/CWE-295/jxbrowser-6.23.1/JxBrowserWithoutCertValidation.expected Normal file
View File

@@ -0,0 +1 @@
| JxBrowserWithoutCertValidationV6_23_1.java:17:27:17:39 | new Browser(...) | This JxBrowser instance may not check HTTPS certificates. |

1
java/ql/test/experimental/query-tests/security/CWE-295/jxbrowser-6.23.1/JxBrowserWithoutCertValidation.qlref Normal file
View File

@@ -0,0 +1 @@
experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.ql

36
java/ql/test/experimental/query-tests/security/CWE-295/jxbrowser-6.23.1/JxBrowserWithoutCertValidationV6_23_1.java Normal file
View File

@@ -0,0 +1,36 @@
import com.teamdev.jxbrowser.chromium.Browser;
import com.teamdev.jxbrowser.chromium.LoadHandler;
import com.teamdev.jxbrowser.chromium.LoadParams;
import com.teamdev.jxbrowser.chromium.CertificateErrorParams;
public class JxBrowserWithoutCertValidationV6_23_1 {
public static void main(String[] args) {
badUsage();
goodUsage();
}
private static void badUsage() {
Browser browser = new Browser();
browser.loadURL("https://example.com");
// no further calls
// BAD: The browser ignores any certificate error by default!
}
private static void goodUsage() {
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");
}
}

1
java/ql/test/experimental/query-tests/security/CWE-295/jxbrowser-6.23.1/options Normal file
View File

@@ -0,0 +1 @@
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/jxbrowser-6.23.1

0
java/ql/test/experimental/query-tests/security/CWE-295/jxbrowser-6.24/JxBrowserWithoutCertValidation.expected Normal file
View File

1
java/ql/test/experimental/query-tests/security/CWE-295/jxbrowser-6.24/JxBrowserWithoutCertValidation.qlref Normal file
View File

@@ -0,0 +1 @@
experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.ql

36
java/ql/test/experimental/query-tests/security/CWE-295/jxbrowser-6.24/JxBrowserWithoutCertValidationV6_24.java Normal file
View File

@@ -0,0 +1,36 @@
import com.teamdev.jxbrowser.chromium.Browser;
import com.teamdev.jxbrowser.chromium.LoadHandler;
import com.teamdev.jxbrowser.chromium.LoadParams;
import com.teamdev.jxbrowser.chromium.CertificateErrorParams;
public class JxBrowserWithoutCertValidationV6_24 {
public static void main(String[] args) {
goodUsage();
goodUsage2();
}
private static void goodUsage() {
Browser browser = new Browser();
browser.loadURL("https://example.com");
// no further calls
// GOOD: On version 6.24 the browser properly validates certificates by default!
}
private static void goodUsage2() {
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");
}
}

1
java/ql/test/experimental/query-tests/security/CWE-295/jxbrowser-6.24/options Normal file
View File

@@ -0,0 +1 @@
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/jxbrowser-6.24

9
java/ql/test/experimental/stubs/jxbrowser-6.23.1/com/teamdev/jxbrowser/chromium/Browser.java Normal file
View File

@@ -0,0 +1,9 @@
package com.teamdev.jxbrowser.chromium;
public class Browser extends java.lang.Object {
public void setLoadHandler(LoadHandler handler) {
}
public void loadURL(String url) {
}
}

5
java/ql/test/experimental/stubs/jxbrowser-6.23.1/com/teamdev/jxbrowser/chromium/CertificateErrorParams.java Normal file
View File

@@ -0,0 +1,5 @@
package com.teamdev.jxbrowser.chromium;
public final class CertificateErrorParams extends Object {
}

7
java/ql/test/experimental/stubs/jxbrowser-6.23.1/com/teamdev/jxbrowser/chromium/LoadHandler.java Normal file
View File

@@ -0,0 +1,7 @@
package com.teamdev.jxbrowser.chromium;
public interface LoadHandler {
boolean onCertificateError(CertificateErrorParams params);
boolean onLoad(LoadParams params);
}

5
java/ql/test/experimental/stubs/jxbrowser-6.23.1/com/teamdev/jxbrowser/chromium/LoadParams.java Normal file
View File

@@ -0,0 +1,5 @@
package com.teamdev.jxbrowser.chromium;
public final class LoadParams extends Object {
}

5
java/ql/test/experimental/stubs/jxbrowser-6.24/com/teamdev/jxbrowser/chromium/BoundsListener.java Normal file
View File

@@ -0,0 +1,5 @@
package com.teamdev.jxbrowser.chromium;
public interface BoundsListener {
}

13
java/ql/test/experimental/stubs/jxbrowser-6.24/com/teamdev/jxbrowser/chromium/Browser.java Normal file
View File

@@ -0,0 +1,13 @@
package com.teamdev.jxbrowser.chromium;
public class Browser extends java.lang.Object {
public void setLoadHandler(LoadHandler handler) {
}
public void loadURL(String url) {
}
public void addBoundsListener(BoundsListener listener) {
}
}

5
java/ql/test/experimental/stubs/jxbrowser-6.24/com/teamdev/jxbrowser/chromium/CertificateErrorParams.java Normal file
View File

@@ -0,0 +1,5 @@
package com.teamdev.jxbrowser.chromium;
public final class CertificateErrorParams extends Object {
}

7
java/ql/test/experimental/stubs/jxbrowser-6.24/com/teamdev/jxbrowser/chromium/LoadHandler.java Normal file
View File

@@ -0,0 +1,7 @@
package com.teamdev.jxbrowser.chromium;
public interface LoadHandler {
boolean onCertificateError(CertificateErrorParams params);
boolean onLoad(LoadParams params);
}

5
java/ql/test/experimental/stubs/jxbrowser-6.24/com/teamdev/jxbrowser/chromium/LoadParams.java Normal file
View File

@@ -0,0 +1,5 @@
package com.teamdev.jxbrowser.chromium;
public final class LoadParams extends Object {
}