From 15a3068f8a0463bbd64d1b1b6c41e1f70c09d94c Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Fri, 30 Apr 2021 16:23:17 +0200 Subject: [PATCH 01/30] Added query for insecure environment configuration RMI JMX (CVE-2016-8735) --- .../CorrectJmxEnvironmentInitialisation.java | 38 +++++ .../CorrectRmiEnvironmentInitialisation.java | 34 ++++ ...secureRmiJmxEnvironmentConfiguration.qhelp | 68 ++++++++ .../InsecureRmiJmxEnvironmentConfiguration.ql | 145 ++++++++++++++++++ ...ureRmiJmxEnvironmentConfiguration.expected | 18 +++ ...nsecureRmiJmxEnvironmentConfiguration.java | 71 +++++++++ ...secureRmiJmxEnvironmentConfiguration.qlref | 1 + 7 files changed, 375 insertions(+) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java create mode 100644 java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java create mode 100644 java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp create mode 100644 java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql create mode 100644 java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected create mode 100644 java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java create mode 100644 java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qlref diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java new file mode 100644 index 000000000000..d2f8e3dbe985 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java @@ -0,0 +1,38 @@ +import java.io.IOException; +import java.lang.management.ManagementFactory; +import java.rmi.registry.LocateRegistry; +import java.util.HashMap; +import java.util.Map; + +import javax.management.MBeanServer; +import javax.management.remote.JMXConnectorServerFactory; +import javax.management.remote.JMXServiceURL; + +public class CorrectJmxInitialisation { + + public void initAndStartJmxServer() throws IOException{ + int jmxPort = 1919; + LocateRegistry.createRegistry(jmxPort); + + /* Restrict the login function to String Objects only (see CVE-2016-3427) */ + Map env = new HashMap(); + // For Java 10+ + String my_filter = "java.lang.String;!*"; // Deny everything but java.lang.String + env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, my_filter); + + /* Old way + env.put("jmx.remote.rmi.server.credential.types", + new String[] { String[].class.getName(), String.class.getName() }); + */ + + MBeanServer beanServer = ManagementFactory.getPlatformMBeanServer(); + + JMXServiceURL jmxUrl = new JMXServiceURL("service:jmx:rmi:///jndi/rmi://localhost:" + jmxPort + "/jmxrmi"); + + // Create JMXConnectorServer in a secure manner + javax.management.remote.JMXConnectorServer connectorServer = JMXConnectorServerFactory + .newJMXConnectorServer(jmxUrl, env, beanServer); + + connectorServer.start(); + } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java new file mode 100644 index 000000000000..b8a28fd8c527 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java @@ -0,0 +1,34 @@ +public class CorrectRmiInitialisation { + public void initAndStartRmiServer(int port, String hostname, boolean local) { + Map env = new HashMap<>(); + + MBeanServerForwarder authzProxy = null; + + env.put("jmx.remote.x.daemon", "true"); + + /* Restrict the login function to String Objects only (see CVE-2016-3427) */ + Map env = new HashMap(); + // For Java 10+ + String my_filter = "java.lang.String;!*"; // Deny everything but java.lang.String + env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, my_filter); + + /* Old way + env.put("jmx.remote.rmi.server.credential.types", + new String[] { String[].class.getName(), String.class.getName() }); + */ + + int rmiPort = Integer.getInteger("com.sun.management.jmxremote.rmi.port", 0); + RMIJRMPServerImpl server = new RMIJRMPServerImpl(rmiPort, + (RMIClientSocketFactory) env.get(RMIConnectorServer.RMI_CLIENT_SOCKET_FACTORY_ATTRIBUTE), + (RMIServerSocketFactory) env.get(RMIConnectorServer.RMI_SERVER_SOCKET_FACTORY_ATTRIBUTE), env); + + JMXServiceURL serviceURL = new JMXServiceURL("rmi", hostname, rmiPort); + + // Create RMI Server + RMIConnectorServer jmxServer = new RMIConnectorServer(serviceURL, env, server, + ManagementFactory.getPlatformMBeanServer()); + + jmxServer.start(); + + } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp new file mode 100644 index 000000000000..a88402b08679 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp @@ -0,0 +1,68 @@ + + + +

An improperly set environment variable during the creation of an RMI or JMX server can lead +to an unauthenticated remote code execution vulnerability. This is due to the fact that the +RMI/JMX server environment allows attackers to supply arbitrary objects to the authentication +method, resulting in the attempted deserialization of an attacker-controlled object. + + + +

During the creation/initialitation of an RMI or JMX server a properly set environment (Map) variable has +to be passed as second parameter. +In order to disallow the deserialization of arbitrary objects the passed environment needs to set a deserialization filter. +Ideally this filter only allows the deserialization to java.lang.String. + +The filter can be configured by setting the key jmx.remote.rmi.server.credentials.filter.pattern (CONST variable RMIConnectorServer.CREDENTIALS_FILTER_PATTERN). +The filter should (ideally) blacklist all classes, and only whitelist java.lang.String for deserialization: ( "java.lang.String;!*"). + +The key-value pair can be set as following: + + +String my_filter = "java.lang.String;!*"; // Deny everything but java.lang.String + +Map env = new HashMap; +env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, my_filter); + + +For applications using < Java 10: + + +// This is deprecated in Java 10+ ! +Map env = new HashMap; +env.put ( + "jmx.remote.rmi.server.credential.types", + new String[]{ + String[].class.getName(), + String.class.getName() + } + ); + + +Please note that the authentication implementation is vulnerable by default. +For this reason an initialitation with a null environment is also vulnerable . + + + +

The following examples show how an RMI or JMX server can be initialized securely. + +

The first example shows how an RMI server can be initialized with a secure environment.

+ + + +

The second example shows how the environment for a JMX server can be initialized securely.

+ + + + + + +
  • OWASP: OWASP Deserialization of untrusted data.
    • +
  • Issue discovered in Tomcat (CVE-2016-8735): OWASP ESAPI.
    • +
  • Vulnerable implementation of the RMI "newClient()" function: Vulnerable Function.
    • +
  • Oracle release notes fixing the issue: Rlease Notes.
    • +
  • Documentation for CREDENTIALS_FILTER_PATTERN
    • +     +     \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql new file mode 100644 index 000000000000..7cfbea9aa1cc --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -0,0 +1,145 @@ +/** + * @name InsecureRmiJmxAuthenticationEnvironment + * @kind path-problem + * @problem.severity error + * @tags security + * external/cwe/cwe-665 + * @precision high + * @id java/insecure-rmi-jmx-server-initalisation + */ + +import java +import semmle.code.java.dataflow.DataFlow +import semmle.code.java.dataflow.DataFlow2 +import semmle.code.java.Maps +import DataFlow::PathGraph +import semmle.code.java.dataflow.NullGuards +import semmle.code.java.dataflow.Nullness + +/** predicate which detects vulnerable Constructors */ +predicate isRmiOrJmxServerCreateConstructor(Constructor constructor) { + constructor.getName() = "RMIConnectorServer" and + constructor + .getDeclaringType() + .hasQualifiedName("javax.management.remote.rmi", "RMIConnectorServer") +} + +/** Predicate which detects vulnerable server creations via methods */ +predicate isRmiOrJmxServerCreateMethod(Method method) { + method.getName() = "newJMXConnectorServer" and + method.getDeclaringType().hasQualifiedName("javax.management.remote", "JMXConnectorServerFactory") +} + +/** + * Models flow from `new HashMap<>()` to a + * `map.put("jmx.remote.rmi.server.credential.types", value)` call. + */ +class MapToPutCredentialstypeConfiguration extends DataFlow2::Configuration { + MapToPutCredentialstypeConfiguration() { this = "MapToPutCredentialstypeConfiguration" } + + override predicate isSource(DataFlow::Node source) { + source.asExpr().(ClassInstanceExpr).getConstructedType() instanceof MapType + } + + override predicate isSink(DataFlow2::Node sink) { putsCredentialtypesKey(sink.asExpr()) } + + /** + * Holds if a `put` call on `qualifier` puts a key match + * into the map. + */ + private predicate putsCredentialtypesKey(Expr qualifier) { + exists(MapPutCall put | + put.getKey().(CompileTimeConstantExpr).getStringValue() = + "jmx.remote.rmi.server.credential.types" or + put.getKey().(CompileTimeConstantExpr).getStringValue() = + "jmx.remote.rmi.server.credentials.filter.pattern" or + put.getKey().toString() = "RMIConnectorServer.CREDENTIAL_TYPES" or // This can probably be solved more nicely + put.getKey().toString() = "RMIConnectorServer.CREDENTIALS_FILTER_PATTERN" // This can probably be solved more nicely + | + put.getQualifier() = qualifier and + put.getMethod().(MapMethod).getReceiverKeyType().getName() = "String" and + put.getMethod().(MapMethod).getReceiverValueType().getName() = "Object" + ) + } +} + +/** Models flow from `new HashMap<>()` to the argument of a `TestConstructor` call. */ +class MapToRmiServerInitConfiguration extends DataFlow::Configuration { + MapToRmiServerInitConfiguration() { this = "MapToRmiServerInitConfiguration" } + + override predicate isSource(DataFlow::Node source) { + source.asExpr().(ClassInstanceExpr).getConstructedType() instanceof MapType + } + + override predicate isSink(DataFlow::Node sink) { + exists(ConstructorCall ccall | + sink.asExpr() = ccall.getArgument(1) and + isRmiOrJmxServerCreateConstructor(ccall.getConstructor()) + ) + or + exists(MethodAccess ma | + sink.asExpr() = ma.getArgument(1) and + isRmiOrJmxServerCreateMethod(ma.getMethod()) + ) + } +} + +/** Models if any JMX/RMI server are initialized with a null environment */ +class FlowServerInitializedWithNullEnv extends DataFlow::Configuration { + FlowServerInitializedWithNullEnv() { this = "FlowServerInitializedWithNullEnv" } + + override predicate isSource(DataFlow::Node source) { any() } + + override predicate isSink(DataFlow::Node sink) { + exists(ConstructorCall ccall | + sink.asExpr() = ccall and + isRmiOrJmxServerCreateConstructor(ccall.getConstructor()) and + ccall.getArgument(1) = alwaysNullExpr() + ) + or + exists(MethodAccess ma | + sink.asExpr() = ma and + isRmiOrJmxServerCreateMethod(ma.getMethod()) and + ma.getArgument(1) = alwaysNullExpr() + ) + } +} + +/** Returns true if within the passed PathNode a "jmx.remote.rmi.server.credential.types" is set. */ +predicate mapFlowContainsCredentialtype(DataFlow::PathNode source) { + exists(MapToPutCredentialstypeConfiguration conf | conf.hasFlow(source.getNode(), _)) +} + +/** Returns result depending if the vulnerability is present due to a) a null environment b) an insecurely set environment map */ +bindingset[source] +string getRmiResult(DataFlow::PathNode source) { + // We got a Map so we have a source and a sink node + if source.getNode().getType() instanceof MapType + then + result = + "RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method." + else + // The environment is not a map so we most likely have a "null" environment and therefore only a sink + result = + "RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks." +} + +/** Predicate returns true for any map flow paths with NO jmx.remote.rmi.server.credential.types set */ +predicate hasVulnerableMapFlow(DataFlow::PathNode source, DataFlow::PathNode sink) { + exists(MapToRmiServerInitConfiguration dataflow | + dataflow.hasFlowPath(source, sink) and + not mapFlowContainsCredentialtype(source) + ) +} + +from + DataFlow::PathNode source, DataFlow::PathNode sink, + FlowServerInitializedWithNullEnv initNullDataflow +where + // Check if server is created with null env + initNullDataflow.hasFlowPath(source, sink) + or + // The map created by `new HashMap()` has to a) flow to the sink and b) there must not exist a (different) sink that would put `"jmx.remote.rmi.server.credential.types"` into `source`. */ + hasVulnerableMapFlow(source, sink) +select sink.getNode(), source, sink, getRmiResult(source), sink.getNode(), "here", source.getNode(), + "source environment 'Map'" diff --git a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected new file mode 100644 index 000000000000..16e321022aeb --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected @@ -0,0 +1,18 @@ +edges +| InsecureRmiServerInitialisation.java:32:31:32:45 | new HashMap(...) : HashMap | InsecureRmiServerInitialisation.java:34:59:34:61 | env | +| InsecureRmiServerInitialisation.java:39:31:39:45 | new HashMap(...) : HashMap | InsecureRmiServerInitialisation.java:43:59:43:61 | env | +| InsecureRmiServerInitialisation.java:57:31:57:45 | new HashMap(...) : HashMap | InsecureRmiServerInitialisation.java:61:59:61:61 | env | +nodes +| InsecureRmiServerInitialisation.java:13:5:13:69 | newJMXConnectorServer(...) | semmle.label | newJMXConnectorServer(...) | +| InsecureRmiServerInitialisation.java:32:31:32:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | +| InsecureRmiServerInitialisation.java:34:59:34:61 | env | semmle.label | env | +| InsecureRmiServerInitialisation.java:39:31:39:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | +| InsecureRmiServerInitialisation.java:43:59:43:61 | env | semmle.label | env | +| InsecureRmiServerInitialisation.java:57:31:57:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | +| InsecureRmiServerInitialisation.java:61:59:61:61 | env | semmle.label | env | +#select +| InsecureRmiServerInitialisation.java:13:5:13:69 | newJMXConnectorServer(...) | InsecureRmiServerInitialisation.java:13:5:13:69 | newJMXConnectorServer(...) | InsecureRmiServerInitialisation.java:13:5:13:69 | newJMXConnectorServer(...) | RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiServerInitialisation.java:13:5:13:69 | newJMXConnectorServer(...) | here | InsecureRmiServerInitialisation.java:13:5:13:69 | newJMXConnectorServer(...) | source environment 'Map' | +| InsecureRmiServerInitialisation.java:34:59:34:61 | env | InsecureRmiServerInitialisation.java:32:31:32:45 | new HashMap(...) : HashMap | InsecureRmiServerInitialisation.java:34:59:34:61 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiServerInitialisation.java:34:59:34:61 | env | here | InsecureRmiServerInitialisation.java:32:31:32:45 | new HashMap(...) | source environment 'Map' | +| InsecureRmiServerInitialisation.java:61:59:61:61 | env | InsecureRmiServerInitialisation.java:57:31:57:45 | new HashMap(...) : HashMap | InsecureRmiServerInitialisation.java:61:59:61:61 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiServerInitialisation.java:61:59:61:61 | env | here | InsecureRmiServerInitialisation.java:57:31:57:45 | new HashMap(...) | source environment 'Map' | + +TODO RMI Server is missing due to import errors (See test java file) diff --git a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java new file mode 100644 index 000000000000..b4e6ba389fdd --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java @@ -0,0 +1,71 @@ +import java.io.IOException; +import javax.management.remote.JMXConnectorServerFactory; + +// import javax.management.remote.rmi.RMIConnectorServer; Importing this throws an error, therefore we can't test this + +import java.util.HashMap; +import java.util.Map; + +public class InsecureRmiServerInitialisation { + + public void initInsecureJmxDueToNullEnv() throws IOException { + // Bad initializing env (arg1) with null + JMXConnectorServerFactory.newJMXConnectorServer(null, null, null); + } + + public void initInsecureRmiDueToNullEnv() throws IOException { + // Bad initializing env (arg1) with null + // new RMIConnectorServer(null, null, null, null); Importing this throws an error, therefore we can't test this + } + + public void initInsecureRmiDueToMissingEnvKeyValue() throws IOException { + // Bad initializing env (arg1) with missing + // "jmx.remote.rmi.server.credential.types" + Map env = new HashMap<>(); + env.put("jmx.remote.x.daemon", "true"); + // new RMIConnectorServer(null, env, null, null); Importing this throws an error, therefore we can't test this + } + + public void initInsecureJmxDueToMissingEnvKeyValue() throws IOException { + // Bad initializing env (arg1) with missing + // "jmx.remote.rmi.server.credential.types" + Map env = new HashMap<>(); + env.put("jmx.remote.x.daemon", "true"); + JMXConnectorServerFactory.newJMXConnectorServer(null, env, null); + } + + public void secureJmxConnnectorServer() throws IOException { + // Good + Map env = new HashMap<>(); + env.put("jmx.remote.x.daemon", "true"); + env.put("jmx.remote.rmi.server.credential.types", + new String[] { String[].class.getName(), String.class.getName() }); + JMXConnectorServerFactory.newJMXConnectorServer(null, env, null); + } + + public void secureRmiConnnectorServer() throws IOException { + // Good + Map env = new HashMap<>(); + env.put("jmx.remote.x.daemon", "true"); + env.put("jmx.remote.rmi.server.credential.types", + new String[] { String[].class.getName(), String.class.getName() }); + // new RMIConnectorServer(null, env, null, null); Importing this throws an error, therefore we can't test this + } + + public void secureeJmxConnectorServerConstants() throws IOException { + // Good + Map env = new HashMap<>(); + env.put("jmx.remote.x.daemon", "true"); + env.put("RMIConnectorServer.SERIAL_FILTER_PATTERN", + new String[] { String[].class.getName(), String.class.getName() }); + JMXConnectorServerFactory.newJMXConnectorServer(null, env, null); + } + public void secureeRmiConnectorServerConstants() throws IOException { + // Good + Map env = new HashMap<>(); + env.put("jmx.remote.x.daemon", "true"); + env.put("RMIConnectorServer.SERIAL_FILTER_PATTERN", + new String[] { String[].class.getName(), String.class.getName() }); + // new RMIConnectorServer(null, env, null, null); Importing this throws an error, therefore we can't test this + } +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qlref b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qlref new file mode 100644 index 000000000000..3b6fc20255fb --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-665/InsecureRmiServerInitialisation.ql \ No newline at end of file From 61d053f6b3f1074a230bc842f998d4aff96b6fed Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Fri, 30 Apr 2021 16:28:17 +0200 Subject: [PATCH 02/30] Fixed missing metadata description --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 1 + 1 file changed, 1 insertion(+) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index 7cfbea9aa1cc..3cc156c2e398 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -1,5 +1,6 @@ /** * @name InsecureRmiJmxAuthenticationEnvironment + * @description This query detects if a JMX/RMI server is created with a potentially dangerous environment, which could lead to code execution through insecure deserialization. * @kind path-problem * @problem.severity error * @tags security From c22eeacbfc4907f1f19e48f2ef990607af80a39f Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Fri, 30 Apr 2021 16:28:56 +0200 Subject: [PATCH 03/30] Fixed accidential double init of variable --- .../CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java index b8a28fd8c527..61d01afa097d 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java +++ b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java @@ -1,7 +1,5 @@ public class CorrectRmiInitialisation { public void initAndStartRmiServer(int port, String hostname, boolean local) { - Map env = new HashMap<>(); - MBeanServerForwarder authzProxy = null; env.put("jmx.remote.x.daemon", "true"); From f28e994121c7eedd12beac862093191ba31b9a7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Tue, 4 May 2021 13:52:47 +0200 Subject: [PATCH 04/30] Update java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp More descriptive (and PC) description. Co-authored-by: Marcono1234 --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp index a88402b08679..678694e7ebf8 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp @@ -16,7 +16,7 @@ In order to disallow the deserialization of arbitrary objects the passed environ Ideally this filter only allows the deserialization to java.lang.String. The filter can be configured by setting the key jmx.remote.rmi.server.credentials.filter.pattern (CONST variable RMIConnectorServer.CREDENTIALS_FILTER_PATTERN). -The filter should (ideally) blacklist all classes, and only whitelist java.lang.String for deserialization: ( "java.lang.String;!*"). +The filter should (ideally) only allow java.lang.String and disallow all other classes for deserialization: ("java.lang.String;!*"). The key-value pair can be set as following: @@ -65,4 +65,4 @@ For this reason an initialitation with a null environment is also v
  • Oracle release notes fixing the issue: Rlease Notes.
  • Documentation for CREDENTIALS_FILTER_PATTERN
    • - \ No newline at end of file + From 7026d82a728ac8713e3487a1c315758a826fb1a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Tue, 4 May 2021 13:53:14 +0200 Subject: [PATCH 05/30] Fixed typo Co-authored-by: Marcono1234 --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp index 678694e7ebf8..670d961e8819 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp @@ -42,7 +42,7 @@ env.put ( Please note that the authentication implementation is vulnerable by default. -For this reason an initialitation with a null environment is also vulnerable . +For this reason an initialization with a null environment is also vulnerable. From 1fd2be3879a242b0a2a5be8d2dff2d2f84bfbfe5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Tue, 4 May 2021 13:57:19 +0200 Subject: [PATCH 06/30] Added more clear reference Co-authored-by: Marcono1234 --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp index 670d961e8819..4ce48b010ca4 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp @@ -63,6 +63,6 @@ For this reason an initialization with a null environment is also v
  • Issue discovered in Tomcat (CVE-2016-8735): OWASP ESAPI.
  • Vulnerable implementation of the RMI "newClient()" function: Vulnerable Function.
  • Oracle release notes fixing the issue: Rlease Notes.
    • -
  • Documentation for CREDENTIALS_FILTER_PATTERN
    • +
  • Java API Specification: RMIConnectorServer.CREDENTIALS_FILTER_PATTERN
    • From 45443baf84f306c44eb472f3b3a933929aa3f1fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Tue, 4 May 2021 13:58:00 +0200 Subject: [PATCH 07/30] Fixed Typo Co-authored-by: Marcono1234 --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index 3cc156c2e398..0f9f18d525d0 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -17,7 +17,7 @@ import DataFlow::PathGraph import semmle.code.java.dataflow.NullGuards import semmle.code.java.dataflow.Nullness -/** predicate which detects vulnerable Constructors */ +/** Predicate which detects vulnerable Constructors */ predicate isRmiOrJmxServerCreateConstructor(Constructor constructor) { constructor.getName() = "RMIConnectorServer" and constructor From 485a3a139a2363b1e1972ba3c4674bd9b21c9251 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Tue, 4 May 2021 13:58:38 +0200 Subject: [PATCH 08/30] Fixed content to confirm with the style guide Co-authored-by: Marcono1234 --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index 0f9f18d525d0..c2c9123e69c2 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -111,7 +111,7 @@ predicate mapFlowContainsCredentialtype(DataFlow::PathNode source) { exists(MapToPutCredentialstypeConfiguration conf | conf.hasFlow(source.getNode(), _)) } -/** Returns result depending if the vulnerability is present due to a) a null environment b) an insecurely set environment map */ +/** Gets a string describing why the application is vulnerable, depending on if the vulnerability is present due to a) a null environment b) an insecurely set environment map */ bindingset[source] string getRmiResult(DataFlow::PathNode source) { // We got a Map so we have a source and a sink node From ab308b5e9e3f94b66a2e0922de44a8fdc193a668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Tue, 4 May 2021 13:59:43 +0200 Subject: [PATCH 09/30] Fix accordance to style guide Co-authored-by: Marcono1234 --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index c2c9123e69c2..a5956dd4f87b 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -6,7 +6,7 @@ * @tags security * external/cwe/cwe-665 * @precision high - * @id java/insecure-rmi-jmx-server-initalisation + * @id java/insecure-rmi-jmx-server-initialization */ import java From 030e2bdd9b5472d28c5e9a98bc1c37a90d154fc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Tue, 4 May 2021 13:59:52 +0200 Subject: [PATCH 10/30] Fix accordance to style guide Co-authored-by: Marcono1234 --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index a5956dd4f87b..006c3be60e32 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -106,7 +106,7 @@ class FlowServerInitializedWithNullEnv extends DataFlow::Configuration { } } -/** Returns true if within the passed PathNode a "jmx.remote.rmi.server.credential.types" is set. */ +/** Holds if within the passed PathNode a "jmx.remote.rmi.server.credential.types" is set. */ predicate mapFlowContainsCredentialtype(DataFlow::PathNode source) { exists(MapToPutCredentialstypeConfiguration conf | conf.hasFlow(source.getNode(), _)) } From c476b6c08890a70409510ba3b0f3b7974795d9dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Tue, 4 May 2021 14:00:01 +0200 Subject: [PATCH 11/30] Fix accordance to style guide Co-authored-by: Marcono1234 --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index 006c3be60e32..3690bfcbd9db 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -125,7 +125,7 @@ string getRmiResult(DataFlow::PathNode source) { "RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks." } -/** Predicate returns true for any map flow paths with NO jmx.remote.rmi.server.credential.types set */ +/** Holds for any map flow paths with **no** jmx.remote.rmi.server.credential.types set */ predicate hasVulnerableMapFlow(DataFlow::PathNode source, DataFlow::PathNode sink) { exists(MapToRmiServerInitConfiguration dataflow | dataflow.hasFlowPath(source, sink) and From 374ed851a0f92c36bb1321166e3d2f117d287257 Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Tue, 4 May 2021 15:12:50 +0200 Subject: [PATCH 12/30] Fixed file reference in test cases --- .../CWE-665/InsecureRmiJmxEnvironmentConfiguration.qlref | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qlref b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qlref index 3b6fc20255fb..de4b67445338 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qlref +++ b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qlref @@ -1 +1 @@ -experimental/Security/CWE/CWE-665/InsecureRmiServerInitialisation.ql \ No newline at end of file +experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql \ No newline at end of file From 787a4ede85471816e80e7cfa49ae32a82ca538ed Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Tue, 4 May 2021 15:33:53 +0200 Subject: [PATCH 13/30] Fixed file reference in test cases --- .../CWE-665/InsecureRmiJmxEnvironmentConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java index b4e6ba389fdd..1c2104218ea0 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java +++ b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java @@ -6,7 +6,7 @@ import java.util.HashMap; import java.util.Map; -public class InsecureRmiServerInitialisation { +public class InsecureRmiJmxEnvironmentConfiguration { public void initInsecureJmxDueToNullEnv() throws IOException { // Bad initializing env (arg1) with null From fd52135f294e9debd37aff328b67b75dbc8adc15 Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Tue, 4 May 2021 15:45:30 +0200 Subject: [PATCH 14/30] Removed unnecessary check for type --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index 3690bfcbd9db..9bd1712a3d7c 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -19,7 +19,6 @@ import semmle.code.java.dataflow.Nullness /** Predicate which detects vulnerable Constructors */ predicate isRmiOrJmxServerCreateConstructor(Constructor constructor) { - constructor.getName() = "RMIConnectorServer" and constructor .getDeclaringType() .hasQualifiedName("javax.management.remote.rmi", "RMIConnectorServer") @@ -38,7 +37,7 @@ predicate isRmiOrJmxServerCreateMethod(Method method) { class MapToPutCredentialstypeConfiguration extends DataFlow2::Configuration { MapToPutCredentialstypeConfiguration() { this = "MapToPutCredentialstypeConfiguration" } - override predicate isSource(DataFlow::Node source) { + override predicate isSource(DataFlow2::Node source) { source.asExpr().(ClassInstanceExpr).getConstructedType() instanceof MapType } From f7437422c190ccf2bcd4d960aa3ffc2cc73838bc Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Tue, 4 May 2021 15:51:40 +0200 Subject: [PATCH 15/30] InstanceOf check instead of comparing classnames --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index 9bd1712a3d7c..b9be4f07cee0 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -57,8 +57,8 @@ class MapToPutCredentialstypeConfiguration extends DataFlow2::Configuration { put.getKey().toString() = "RMIConnectorServer.CREDENTIALS_FILTER_PATTERN" // This can probably be solved more nicely | put.getQualifier() = qualifier and - put.getMethod().(MapMethod).getReceiverKeyType().getName() = "String" and - put.getMethod().(MapMethod).getReceiverValueType().getName() = "Object" + put.getMethod().(MapMethod).getReceiverKeyType() instanceof TypeString and + put.getMethod().(MapMethod).getReceiverValueType() instanceof TypeObject ) } } From 81363a88437b2c289b72e96a2195d69d7473248d Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Tue, 4 May 2021 15:57:47 +0200 Subject: [PATCH 16/30] Some better (and more styleguide compliant) descriptions within the query. --- .../CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index b9be4f07cee0..97bb4f7ed0c4 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -63,7 +63,10 @@ class MapToPutCredentialstypeConfiguration extends DataFlow2::Configuration { } } -/** Models flow from `new HashMap<>()` to the argument of a `TestConstructor` call. */ +/** + * Models flow from `new HashMap<>()` variable which is later used as environment during + * a JMX/RMI server initalitation with `newJMXConnectorServer(...)` or `RMIConnectorServer(...)` + */ class MapToRmiServerInitConfiguration extends DataFlow::Configuration { MapToRmiServerInitConfiguration() { this = "MapToRmiServerInitConfiguration" } @@ -139,7 +142,11 @@ where // Check if server is created with null env initNullDataflow.hasFlowPath(source, sink) or - // The map created by `new HashMap()` has to a) flow to the sink and b) there must not exist a (different) sink that would put `"jmx.remote.rmi.server.credential.types"` into `source`. */ + /* + * The map created by `new HashMap()` has to a) flow to the sink and + * b) there must not exist a (different) sink that would put `"jmx.remote.rmi.server.credential.types"` into `source`. + */ + hasVulnerableMapFlow(source, sink) select sink.getNode(), source, sink, getRmiResult(source), sink.getNode(), "here", source.getNode(), "source environment 'Map'" From 152f4862ec4659c0d133f6c5f9d5b9659409ddfa Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Tue, 4 May 2021 16:10:15 +0200 Subject: [PATCH 17/30] Reworked the references a bit --- .../CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp index 4ce48b010ca4..fe75d4531659 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp @@ -59,10 +59,10 @@ For this reason an initialization with a null environment is also v     -
  • OWASP: OWASP Deserialization of untrusted data.
    • +
  • Deserialization of arbitrary objects could lead to remote code execution as discribed following: OWASP Deserialization of untrusted data.
  • Issue discovered in Tomcat (CVE-2016-8735): OWASP ESAPI.
    • -
  • Vulnerable implementation of the RMI "newClient()" function: Vulnerable Function.
  • Oracle release notes fixing the issue: Rlease Notes.
    • -
  • Java API Specification: RMIConnectorServer.CREDENTIALS_FILTER_PATTERN
    • +
  • Java 10 API specification for RMIConnectorServer.CREDENTIALS_FILTER_PATTERN
    • +
  • The Java API specification forCREDENTIAL_TYPES. Please note that this field is deprecated since Java 10.
    •  From 65642df1a00b3b514db0a93f87cd941ed46c013a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Tue, 4 May 2021 17:28:34 +0200 Subject: [PATCH 18/30] Apply suggestions from code review for help text Co-authored-by: Marcono1234 --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp | 4 ++-- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp index fe75d4531659..58391d7dc466 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp @@ -59,10 +59,10 @@ For this reason an initialization with a null environment is also v -
  • Deserialization of arbitrary objects could lead to remote code execution as discribed following: OWASP Deserialization of untrusted data.
    • +
  • Deserialization of arbitrary objects could lead to remote code execution as described following: OWASP Deserialization of untrusted data.
  • Issue discovered in Tomcat (CVE-2016-8735): OWASP ESAPI.
  • Oracle release notes fixing the issue: Rlease Notes.
  • Java 10 API specification for RMIConnectorServer.CREDENTIALS_FILTER_PATTERN
    • -
  • The Java API specification forCREDENTIAL_TYPES. Please note that this field is deprecated since Java 10.
    • +
  • The Java API specification for RMIConnectorServer.CREDENTIAL_TYPES. Please note that this field is deprecated since Java 10.
    •  diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index 97bb4f7ed0c4..7448f49784ef 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -65,7 +65,7 @@ class MapToPutCredentialstypeConfiguration extends DataFlow2::Configuration { /** * Models flow from `new HashMap<>()` variable which is later used as environment during - * a JMX/RMI server initalitation with `newJMXConnectorServer(...)` or `RMIConnectorServer(...)` + * a JMX/RMI server initialization with `newJMXConnectorServer(...)` or `RMIConnectorServer(...)` */ class MapToRmiServerInitConfiguration extends DataFlow::Configuration { MapToRmiServerInitConfiguration() { this = "MapToRmiServerInitConfiguration" } From a65481d24ba0251de3cabfce2d2e97e48ff37516 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Tue, 4 May 2021 17:30:49 +0200 Subject: [PATCH 19/30] Apply suggestions from code review more precise help text --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp index 58391d7dc466..4bd71587177b 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp @@ -61,7 +61,7 @@ For this reason an initialization with a null environment is also v
  • Deserialization of arbitrary objects could lead to remote code execution as described following: OWASP Deserialization of untrusted data.
  • Issue discovered in Tomcat (CVE-2016-8735): OWASP ESAPI.
    • -
  • Oracle release notes fixing the issue: Rlease Notes.
    • +
  • Oracle release notes: New attribute for JMX RMI JRMP servers.
  • Java 10 API specification for RMIConnectorServer.CREDENTIALS_FILTER_PATTERN
  • The Java API specification for RMIConnectorServer.CREDENTIAL_TYPES. Please note that this field is deprecated since Java 10.
    •  From e7021ffbee4f5834868e70f51232c19dc3ebf760 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Tue, 25 May 2021 12:53:47 +0200 Subject: [PATCH 20/30] Apply suggestions from code review More clear or precise wording within the documentation Co-authored-by: Chris Smowton --- .../CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java | 2 +- .../CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java | 2 +- .../CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp | 6 +++--- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java index d2f8e3dbe985..4152e97ddd05 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java +++ b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java @@ -20,7 +20,7 @@ public void initAndStartJmxServer() throws IOException{ String my_filter = "java.lang.String;!*"; // Deny everything but java.lang.String env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, my_filter); - /* Old way + /* Java 9 or below: env.put("jmx.remote.rmi.server.credential.types", new String[] { String[].class.getName(), String.class.getName() }); */ diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java index 61d01afa097d..724427ef5977 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java +++ b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java @@ -10,7 +10,7 @@ public void initAndStartRmiServer(int port, String hostname, boolean local) { String my_filter = "java.lang.String;!*"; // Deny everything but java.lang.String env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, my_filter); - /* Old way + /* Java 9 or below env.put("jmx.remote.rmi.server.credential.types", new String[] { String[].class.getName(), String.class.getName() }); */ diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp index 4bd71587177b..83bf7519ebd8 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp @@ -4,7 +4,7 @@

    An improperly set environment variable during the creation of an RMI or JMX server can lead -to an unauthenticated remote code execution vulnerability. This is due to the fact that the +to an unauthenticated remote code execution vulnerability. This is because the RMI/JMX server environment allows attackers to supply arbitrary objects to the authentication method, resulting in the attempted deserialization of an attacker-controlled object. @@ -15,7 +15,7 @@ to be passed as second parameter. In order to disallow the deserialization of arbitrary objects the passed environment needs to set a deserialization filter. Ideally this filter only allows the deserialization to java.lang.String. -The filter can be configured by setting the key jmx.remote.rmi.server.credentials.filter.pattern (CONST variable RMIConnectorServer.CREDENTIALS_FILTER_PATTERN). +The filter can be configured by setting the key jmx.remote.rmi.server.credentials.filter.pattern (given by the constant RMIConnectorServer.CREDENTIALS_FILTER_PATTERN). The filter should (ideally) only allow java.lang.String and disallow all other classes for deserialization: ("java.lang.String;!*"). The key-value pair can be set as following: @@ -27,7 +27,7 @@ Map env = new HashMap; env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, my_filter); -For applications using < Java 10: +For applications using Java 9 or below: // This is deprecated in Java 10+ ! diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index 7448f49784ef..9bf195548c2a 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -1,6 +1,6 @@ /** * @name InsecureRmiJmxAuthenticationEnvironment - * @description This query detects if a JMX/RMI server is created with a potentially dangerous environment, which could lead to code execution through insecure deserialization. + * @description Creating a JMX/RMI server could lead to code execution through insecure deserialization if its environment does not restrict the types that can be deserialized. * @kind path-problem * @problem.severity error * @tags security @@ -17,14 +17,14 @@ import DataFlow::PathGraph import semmle.code.java.dataflow.NullGuards import semmle.code.java.dataflow.Nullness -/** Predicate which detects vulnerable Constructors */ +/** Holds if `constructor` instantiates an RMI or JMX server. */ predicate isRmiOrJmxServerCreateConstructor(Constructor constructor) { constructor .getDeclaringType() .hasQualifiedName("javax.management.remote.rmi", "RMIConnectorServer") } -/** Predicate which detects vulnerable server creations via methods */ +/** Holds if `method` creates an RMI or JMX server. */ predicate isRmiOrJmxServerCreateMethod(Method method) { method.getName() = "newJMXConnectorServer" and method.getDeclaringType().hasQualifiedName("javax.management.remote", "JMXConnectorServerFactory") From f44b97c1c3b7109f7647d9761d3e56eb08f7ad63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Tue, 25 May 2021 13:03:07 +0200 Subject: [PATCH 21/30] Apply suggestions from code review Improved variable naming in examples and some documentation clearup Co-authored-by: Chris Smowton --- .../CWE-665/CorrectJmxEnvironmentInitialisation.java | 4 ++-- .../CWE-665/CorrectRmiEnvironmentInitialisation.java | 4 ++-- .../InsecureRmiJmxEnvironmentConfiguration.qhelp | 10 ++++------ 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java index 4152e97ddd05..0212687862a4 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java +++ b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java @@ -17,8 +17,8 @@ public void initAndStartJmxServer() throws IOException{ /* Restrict the login function to String Objects only (see CVE-2016-3427) */ Map env = new HashMap(); // For Java 10+ - String my_filter = "java.lang.String;!*"; // Deny everything but java.lang.String - env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, my_filter); + String stringsOnlyFilter = "java.lang.String;!*"; // Deny everything but java.lang.String + env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, stringsOnlyFilter); /* Java 9 or below: env.put("jmx.remote.rmi.server.credential.types", diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java index 724427ef5977..400038b68538 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java +++ b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java @@ -7,8 +7,8 @@ public void initAndStartRmiServer(int port, String hostname, boolean local) { /* Restrict the login function to String Objects only (see CVE-2016-3427) */ Map env = new HashMap(); // For Java 10+ - String my_filter = "java.lang.String;!*"; // Deny everything but java.lang.String - env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, my_filter); + String stringsOnlyFilter = "java.lang.String;!*"; // Deny everything but java.lang.String + env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, stringsOnlyFilter); /* Java 9 or below env.put("jmx.remote.rmi.server.credential.types", diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp index 83bf7519ebd8..f20b55d1a1a9 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp @@ -10,10 +10,8 @@ method, resulting in the attempted deserialization of an attacker-controlled obj -

    During the creation/initialitation of an RMI or JMX server a properly set environment (Map) variable has -to be passed as second parameter. -In order to disallow the deserialization of arbitrary objects the passed environment needs to set a deserialization filter. -Ideally this filter only allows the deserialization to java.lang.String. +

    During the creation/initialization of an RMI or JMX server an environment should be supplied that sets a deserialization filter. +Ideally this filter only allows the deserialization of java.lang.String. The filter can be configured by setting the key jmx.remote.rmi.server.credentials.filter.pattern (given by the constant RMIConnectorServer.CREDENTIALS_FILTER_PATTERN). The filter should (ideally) only allow java.lang.String and disallow all other classes for deserialization: ("java.lang.String;!*"). @@ -21,10 +19,10 @@ The filter should (ideally) only allow java.lang.String and disallow all other c The key-value pair can be set as following: -String my_filter = "java.lang.String;!*"; // Deny everything but java.lang.String +String stringsOnlyFilter = "java.lang.String;!*"; // Deny everything but java.lang.String Map env = new HashMap; -env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, my_filter); +env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, stringsOnlyFilter); For applications using Java 9 or below: From 59ebe08c78b3dfa39abbf4df58560e82142ef3bc Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Tue, 25 May 2021 16:40:41 +0200 Subject: [PATCH 22/30] Added stup for RMIConnectorServer for valid test case --- ...ureRmiJmxEnvironmentConfiguration.expected | 44 ++++++++++++------- ...nsecureRmiJmxEnvironmentConfiguration.java | 12 ++--- .../query-tests/security/CWE-665/options | 1 + .../remote/rmi/RMIConnectorServer.java | 27 ++++++++++++ .../management/remote/rmi/RMIServerImpl.java | 10 +++++ 5 files changed, 74 insertions(+), 20 deletions(-) create mode 100644 java/ql/test/experimental/query-tests/security/CWE-665/options create mode 100644 java/ql/test/experimental/stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java create mode 100644 java/ql/test/experimental/stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIServerImpl.java diff --git a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected index 16e321022aeb..b22542457f33 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected @@ -1,18 +1,32 @@ edges -| InsecureRmiServerInitialisation.java:32:31:32:45 | new HashMap(...) : HashMap | InsecureRmiServerInitialisation.java:34:59:34:61 | env | -| InsecureRmiServerInitialisation.java:39:31:39:45 | new HashMap(...) : HashMap | InsecureRmiServerInitialisation.java:43:59:43:61 | env | -| InsecureRmiServerInitialisation.java:57:31:57:45 | new HashMap(...) : HashMap | InsecureRmiServerInitialisation.java:61:59:61:61 | env | +| ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:23:12:23:29 | this [post update] : RMIConnectorServer | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | +| InsecureRmiJmxEnvironmentConfiguration.java:25:31:25:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | +| InsecureRmiJmxEnvironmentConfiguration.java:33:31:33:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | +| InsecureRmiJmxEnvironmentConfiguration.java:40:31:40:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:44:59:44:61 | env | +| InsecureRmiJmxEnvironmentConfiguration.java:49:31:49:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:53:34:53:36 | env | +| InsecureRmiJmxEnvironmentConfiguration.java:58:31:58:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:62:59:62:61 | env | +| InsecureRmiJmxEnvironmentConfiguration.java:67:31:67:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:71:34:71:36 | env | nodes -| InsecureRmiServerInitialisation.java:13:5:13:69 | newJMXConnectorServer(...) | semmle.label | newJMXConnectorServer(...) | -| InsecureRmiServerInitialisation.java:32:31:32:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | -| InsecureRmiServerInitialisation.java:34:59:34:61 | env | semmle.label | env | -| InsecureRmiServerInitialisation.java:39:31:39:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | -| InsecureRmiServerInitialisation.java:43:59:43:61 | env | semmle.label | env | -| InsecureRmiServerInitialisation.java:57:31:57:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | -| InsecureRmiServerInitialisation.java:61:59:61:61 | env | semmle.label | env | +| ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:23:12:23:29 | this [post update] : RMIConnectorServer | semmle.label | this [post update] : RMIConnectorServer | +| InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | semmle.label | newJMXConnectorServer(...) | +| InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | semmle.label | new RMIConnectorServer(...) | +| InsecureRmiJmxEnvironmentConfiguration.java:25:31:25:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | +| InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | semmle.label | env | +| InsecureRmiJmxEnvironmentConfiguration.java:33:31:33:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | +| InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | semmle.label | env | +| InsecureRmiJmxEnvironmentConfiguration.java:40:31:40:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | +| InsecureRmiJmxEnvironmentConfiguration.java:44:59:44:61 | env | semmle.label | env | +| InsecureRmiJmxEnvironmentConfiguration.java:49:31:49:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | +| InsecureRmiJmxEnvironmentConfiguration.java:53:34:53:36 | env | semmle.label | env | +| InsecureRmiJmxEnvironmentConfiguration.java:58:31:58:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | +| InsecureRmiJmxEnvironmentConfiguration.java:62:59:62:61 | env | semmle.label | env | +| InsecureRmiJmxEnvironmentConfiguration.java:67:31:67:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | +| InsecureRmiJmxEnvironmentConfiguration.java:71:34:71:36 | env | semmle.label | env | #select -| InsecureRmiServerInitialisation.java:13:5:13:69 | newJMXConnectorServer(...) | InsecureRmiServerInitialisation.java:13:5:13:69 | newJMXConnectorServer(...) | InsecureRmiServerInitialisation.java:13:5:13:69 | newJMXConnectorServer(...) | RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiServerInitialisation.java:13:5:13:69 | newJMXConnectorServer(...) | here | InsecureRmiServerInitialisation.java:13:5:13:69 | newJMXConnectorServer(...) | source environment 'Map' | -| InsecureRmiServerInitialisation.java:34:59:34:61 | env | InsecureRmiServerInitialisation.java:32:31:32:45 | new HashMap(...) : HashMap | InsecureRmiServerInitialisation.java:34:59:34:61 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiServerInitialisation.java:34:59:34:61 | env | here | InsecureRmiServerInitialisation.java:32:31:32:45 | new HashMap(...) | source environment 'Map' | -| InsecureRmiServerInitialisation.java:61:59:61:61 | env | InsecureRmiServerInitialisation.java:57:31:57:45 | new HashMap(...) : HashMap | InsecureRmiServerInitialisation.java:61:59:61:61 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiServerInitialisation.java:61:59:61:61 | env | here | InsecureRmiServerInitialisation.java:57:31:57:45 | new HashMap(...) | source environment 'Map' | - -TODO RMI Server is missing due to import errors (See test java file) +| InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | here | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | source environment 'Map' | +| InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:23:12:23:29 | this [post update] : RMIConnectorServer | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | here | ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:23:12:23:29 | this [post update] | source environment 'Map' | +| InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | here | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | source environment 'Map' | +| InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | InsecureRmiJmxEnvironmentConfiguration.java:25:31:25:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | here | InsecureRmiJmxEnvironmentConfiguration.java:25:31:25:45 | new HashMap(...) | source environment 'Map' | +| InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | InsecureRmiJmxEnvironmentConfiguration.java:33:31:33:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | here | InsecureRmiJmxEnvironmentConfiguration.java:33:31:33:45 | new HashMap(...) | source environment 'Map' | +| InsecureRmiJmxEnvironmentConfiguration.java:62:59:62:61 | env | InsecureRmiJmxEnvironmentConfiguration.java:58:31:58:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:62:59:62:61 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiJmxEnvironmentConfiguration.java:62:59:62:61 | env | here | InsecureRmiJmxEnvironmentConfiguration.java:58:31:58:45 | new HashMap(...) | source environment 'Map' | +| InsecureRmiJmxEnvironmentConfiguration.java:71:34:71:36 | env | InsecureRmiJmxEnvironmentConfiguration.java:67:31:67:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:71:34:71:36 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiJmxEnvironmentConfiguration.java:71:34:71:36 | env | here | InsecureRmiJmxEnvironmentConfiguration.java:67:31:67:45 | new HashMap(...) | source environment 'Map' | diff --git a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java index 1c2104218ea0..b58d6841d3d3 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java +++ b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java @@ -1,7 +1,7 @@ import java.io.IOException; import javax.management.remote.JMXConnectorServerFactory; -// import javax.management.remote.rmi.RMIConnectorServer; Importing this throws an error, therefore we can't test this +import javax.management.remote.rmi.RMIConnectorServer; import java.util.HashMap; import java.util.Map; @@ -15,7 +15,8 @@ public void initInsecureJmxDueToNullEnv() throws IOException { public void initInsecureRmiDueToNullEnv() throws IOException { // Bad initializing env (arg1) with null - // new RMIConnectorServer(null, null, null, null); Importing this throws an error, therefore we can't test this + new RMIConnectorServer(null, null, null, null); + } public void initInsecureRmiDueToMissingEnvKeyValue() throws IOException { @@ -23,7 +24,7 @@ public void initInsecureRmiDueToMissingEnvKeyValue() throws IOException { // "jmx.remote.rmi.server.credential.types" Map env = new HashMap<>(); env.put("jmx.remote.x.daemon", "true"); - // new RMIConnectorServer(null, env, null, null); Importing this throws an error, therefore we can't test this + new RMIConnectorServer(null, env, null, null); } public void initInsecureJmxDueToMissingEnvKeyValue() throws IOException { @@ -49,7 +50,7 @@ public void secureRmiConnnectorServer() throws IOException { env.put("jmx.remote.x.daemon", "true"); env.put("jmx.remote.rmi.server.credential.types", new String[] { String[].class.getName(), String.class.getName() }); - // new RMIConnectorServer(null, env, null, null); Importing this throws an error, therefore we can't test this + new RMIConnectorServer(null, env, null, null); } public void secureeJmxConnectorServerConstants() throws IOException { @@ -60,12 +61,13 @@ public void secureeJmxConnectorServerConstants() throws IOException { new String[] { String[].class.getName(), String.class.getName() }); JMXConnectorServerFactory.newJMXConnectorServer(null, env, null); } + public void secureeRmiConnectorServerConstants() throws IOException { // Good Map env = new HashMap<>(); env.put("jmx.remote.x.daemon", "true"); env.put("RMIConnectorServer.SERIAL_FILTER_PATTERN", new String[] { String[].class.getName(), String.class.getName() }); - // new RMIConnectorServer(null, env, null, null); Importing this throws an error, therefore we can't test this + new RMIConnectorServer(null, env, null, null); } } diff --git a/java/ql/test/experimental/query-tests/security/CWE-665/options b/java/ql/test/experimental/query-tests/security/CWE-665/options new file mode 100644 index 000000000000..7fc584f11e39 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-665/options @@ -0,0 +1 @@ +//semmle-extractor-options: --javac-args -source 11 -target 11 -cp ${testdir}/../../../../experimental/stubs/javax-management-remote-rmi-0.0.1 diff --git a/java/ql/test/experimental/stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java b/java/ql/test/experimental/stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java new file mode 100644 index 000000000000..677d2e49afef --- /dev/null +++ b/java/ql/test/experimental/stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java @@ -0,0 +1,27 @@ +package javax.management.remote.rmi; + +import java.io.IOException; +import java.util.Map; +import java.io.IOException; +import javax.management.remote.JMXConnectorServer; +import javax.management.remote.JMXServiceURL; +import javax.management.MBeanServer; +import javax.management.remote.rmi.RMIServerImpl; +//import javax.management.remote.JMXConnectorServer; + +//public class RMIConnectorServerTEST extends JMXConnectorServer{ +public class RMIConnectorServer extends java.lang.Object { + public RMIConnectorServer(JMXServiceURL url, Map environment) throws IOException { + // stub; + } + + public RMIConnectorServer(JMXServiceURL url, Map environment, MBeanServer mbeanServer) + throws IOException { + // stub; + } + + public RMIConnectorServer(JMXServiceURL url, Map environment, RMIServerImpl rmiServerImpl, + MBeanServer mbeanServer) throws IOException { + // stub; + } +} diff --git a/java/ql/test/experimental/stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIServerImpl.java b/java/ql/test/experimental/stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIServerImpl.java new file mode 100644 index 000000000000..e863a55a33ba --- /dev/null +++ b/java/ql/test/experimental/stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIServerImpl.java @@ -0,0 +1,10 @@ +package javax.management.remote.rmi; + +import java.util.Map; + +public class RMIServerImpl { + public RMIServerImpl(Map env) { + // stub; + } + +} From 75f6ec1f0dabaef7b26c3ccfd8cca28edf21f129 Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Tue, 25 May 2021 17:08:58 +0200 Subject: [PATCH 23/30] Updated test cases to include test for java10+ CREDENTIALS_FILTER_PATTERN constant --- ...ureRmiJmxEnvironmentConfiguration.expected | 14 +++++---- ...nsecureRmiJmxEnvironmentConfiguration.java | 30 +++++++++++++++---- .../remote/rmi/RMIConnectorServer.java | 3 ++ 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected index b22542457f33..fc3cb78662e0 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected @@ -1,13 +1,15 @@ edges -| ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:23:12:23:29 | this [post update] : RMIConnectorServer | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | +| ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:26:12:26:29 | this [post update] : RMIConnectorServer | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | | InsecureRmiJmxEnvironmentConfiguration.java:25:31:25:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | | InsecureRmiJmxEnvironmentConfiguration.java:33:31:33:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | | InsecureRmiJmxEnvironmentConfiguration.java:40:31:40:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:44:59:44:61 | env | | InsecureRmiJmxEnvironmentConfiguration.java:49:31:49:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:53:34:53:36 | env | | InsecureRmiJmxEnvironmentConfiguration.java:58:31:58:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:62:59:62:61 | env | | InsecureRmiJmxEnvironmentConfiguration.java:67:31:67:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:71:34:71:36 | env | +| InsecureRmiJmxEnvironmentConfiguration.java:76:31:76:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:80:59:80:61 | env | +| InsecureRmiJmxEnvironmentConfiguration.java:85:31:85:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:89:34:89:36 | env | nodes -| ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:23:12:23:29 | this [post update] : RMIConnectorServer | semmle.label | this [post update] : RMIConnectorServer | +| ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:26:12:26:29 | this [post update] : RMIConnectorServer | semmle.label | this [post update] : RMIConnectorServer | | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | semmle.label | newJMXConnectorServer(...) | | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | semmle.label | new RMIConnectorServer(...) | | InsecureRmiJmxEnvironmentConfiguration.java:25:31:25:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | @@ -22,11 +24,13 @@ nodes | InsecureRmiJmxEnvironmentConfiguration.java:62:59:62:61 | env | semmle.label | env | | InsecureRmiJmxEnvironmentConfiguration.java:67:31:67:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | | InsecureRmiJmxEnvironmentConfiguration.java:71:34:71:36 | env | semmle.label | env | +| InsecureRmiJmxEnvironmentConfiguration.java:76:31:76:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | +| InsecureRmiJmxEnvironmentConfiguration.java:80:59:80:61 | env | semmle.label | env | +| InsecureRmiJmxEnvironmentConfiguration.java:85:31:85:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | +| InsecureRmiJmxEnvironmentConfiguration.java:89:34:89:36 | env | semmle.label | env | #select | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | here | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | source environment 'Map' | -| InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:23:12:23:29 | this [post update] : RMIConnectorServer | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | here | ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:23:12:23:29 | this [post update] | source environment 'Map' | +| InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:26:12:26:29 | this [post update] : RMIConnectorServer | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | here | ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:26:12:26:29 | this [post update] | source environment 'Map' | | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | here | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | source environment 'Map' | | InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | InsecureRmiJmxEnvironmentConfiguration.java:25:31:25:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | here | InsecureRmiJmxEnvironmentConfiguration.java:25:31:25:45 | new HashMap(...) | source environment 'Map' | | InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | InsecureRmiJmxEnvironmentConfiguration.java:33:31:33:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | here | InsecureRmiJmxEnvironmentConfiguration.java:33:31:33:45 | new HashMap(...) | source environment 'Map' | -| InsecureRmiJmxEnvironmentConfiguration.java:62:59:62:61 | env | InsecureRmiJmxEnvironmentConfiguration.java:58:31:58:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:62:59:62:61 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiJmxEnvironmentConfiguration.java:62:59:62:61 | env | here | InsecureRmiJmxEnvironmentConfiguration.java:58:31:58:45 | new HashMap(...) | source environment 'Map' | -| InsecureRmiJmxEnvironmentConfiguration.java:71:34:71:36 | env | InsecureRmiJmxEnvironmentConfiguration.java:67:31:67:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:71:34:71:36 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiJmxEnvironmentConfiguration.java:71:34:71:36 | env | here | InsecureRmiJmxEnvironmentConfiguration.java:67:31:67:45 | new HashMap(...) | source environment 'Map' | diff --git a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java index b58d6841d3d3..67ba5c89ea80 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java +++ b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java @@ -53,21 +53,39 @@ public void secureRmiConnnectorServer() throws IOException { new RMIConnectorServer(null, env, null, null); } - public void secureeJmxConnectorServerConstants() throws IOException { + public void secureeJmxConnectorServerConstants1() throws IOException { // Good Map env = new HashMap<>(); env.put("jmx.remote.x.daemon", "true"); - env.put("RMIConnectorServer.SERIAL_FILTER_PATTERN", - new String[] { String[].class.getName(), String.class.getName() }); + env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, "java.lang.String;!*"); // Deny everything but + // java.lang.String JMXConnectorServerFactory.newJMXConnectorServer(null, env, null); } - public void secureeRmiConnectorServerConstants() throws IOException { + public void secureeRmiConnectorServerConstants1() throws IOException { // Good Map env = new HashMap<>(); env.put("jmx.remote.x.daemon", "true"); - env.put("RMIConnectorServer.SERIAL_FILTER_PATTERN", - new String[] { String[].class.getName(), String.class.getName() }); + String stringsOnlyFilter = "java.lang.String;!*"; // Deny everything but java.lang.String + env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, stringsOnlyFilter); + new RMIConnectorServer(null, env, null, null); + } + + public void secureeJmxConnectorServerConstants2() throws IOException { + // Good + Map env = new HashMap<>(); + env.put("jmx.remote.x.daemon", "true"); + env.put("jmx.remote.rmi.server.credentials.filter.pattern", "java.lang.String;!*"); // Deny everything but + // java.lang.String + JMXConnectorServerFactory.newJMXConnectorServer(null, env, null); + } + + public void secureeRmiConnectorServerConstants2() throws IOException { + // Good + Map env = new HashMap<>(); + env.put("jmx.remote.x.daemon", "true"); + String stringsOnlyFilter = "java.lang.String;!*"; // Deny everything but java.lang.String + env.put("jmx.remote.rmi.server.credentials.filter.pattern", stringsOnlyFilter); new RMIConnectorServer(null, env, null, null); } } diff --git a/java/ql/test/experimental/stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java b/java/ql/test/experimental/stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java index 677d2e49afef..0833611e02c8 100644 --- a/java/ql/test/experimental/stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java +++ b/java/ql/test/experimental/stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java @@ -11,6 +11,9 @@ //public class RMIConnectorServerTEST extends JMXConnectorServer{ public class RMIConnectorServer extends java.lang.Object { + + public static final String CREDENTIALS_FILTER_PATTERN = "jmx.remote.rmi.server.credentials.filter.pattern"; + public RMIConnectorServer(JMXServiceURL url, Map environment) throws IOException { // stub; } From 4ddf4558a76792036eac5309afc5e90c53541ac5 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Fri, 4 Jun 2021 12:09:52 +0100 Subject: [PATCH 24/30] Merged simplified query --- .../InsecureRmiJmxEnvironmentConfiguration.ql | 124 +++++------------- ...ureRmiJmxEnvironmentConfiguration.expected | 40 +----- ...nsecureRmiJmxEnvironmentConfiguration.java | 6 +- .../query-tests/security/CWE-665/options | 2 +- .../stubs/rmi-remote-0.0.0/README | 1 + .../management/remote/rmi/RMIConnection.java | 6 + .../remote/rmi/RMIConnectorServer.java | 34 +++++ .../management/remote/rmi/RMIServer.java | 3 + .../management/remote/rmi/RMIServerImpl.java | 12 ++ 9 files changed, 93 insertions(+), 135 deletions(-) create mode 100644 java/ql/test/experimental/stubs/rmi-remote-0.0.0/README create mode 100644 java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIConnection.java create mode 100644 java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIConnectorServer.java create mode 100644 java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIServer.java create mode 100644 java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIServerImpl.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index 9bf195548c2a..eab6f1f77179 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -1,7 +1,7 @@ /** * @name InsecureRmiJmxAuthenticationEnvironment - * @description Creating a JMX/RMI server could lead to code execution through insecure deserialization if its environment does not restrict the types that can be deserialized. - * @kind path-problem + * @description This query detects if a JMX/RMI server is created with a potentially dangerous environment, which could lead to code execution through insecure deserialization. + * @kind problem * @problem.severity error * @tags security * external/cwe/cwe-665 @@ -11,11 +11,7 @@ import java import semmle.code.java.dataflow.DataFlow -import semmle.code.java.dataflow.DataFlow2 import semmle.code.java.Maps -import DataFlow::PathGraph -import semmle.code.java.dataflow.NullGuards -import semmle.code.java.dataflow.Nullness /** Holds if `constructor` instantiates an RMI or JMX server. */ predicate isRmiOrJmxServerCreateConstructor(Constructor constructor) { @@ -31,17 +27,23 @@ predicate isRmiOrJmxServerCreateMethod(Method method) { } /** - * Models flow from `new HashMap<>()` to a - * `map.put("jmx.remote.rmi.server.credential.types", value)` call. + * Models flow from the qualifier of a + * `map.put("jmx.remote.rmi.server.credential.types", value)` call + * to an RMI or JMX initialisation call. */ -class MapToPutCredentialstypeConfiguration extends DataFlow2::Configuration { - MapToPutCredentialstypeConfiguration() { this = "MapToPutCredentialstypeConfiguration" } +class SafeFlow extends DataFlow::Configuration { + SafeFlow() { this = "MapToPutCredentialstypeConfiguration" } - override predicate isSource(DataFlow2::Node source) { - source.asExpr().(ClassInstanceExpr).getConstructedType() instanceof MapType - } + override predicate isSource(DataFlow::Node source) { putsCredentialtypesKey(source.asExpr()) } - override predicate isSink(DataFlow2::Node sink) { putsCredentialtypesKey(sink.asExpr()) } + override predicate isSink(DataFlow::Node sink) { + exists(Call c | + isRmiOrJmxServerCreateConstructor(c.getCallee()) or + isRmiOrJmxServerCreateMethod(c.getCallee()) + | + sink.asExpr() = c.getArgument(1) + ) + } /** * Holds if a `put` call on `qualifier` puts a key match @@ -53,8 +55,11 @@ class MapToPutCredentialstypeConfiguration extends DataFlow2::Configuration { "jmx.remote.rmi.server.credential.types" or put.getKey().(CompileTimeConstantExpr).getStringValue() = "jmx.remote.rmi.server.credentials.filter.pattern" or - put.getKey().toString() = "RMIConnectorServer.CREDENTIAL_TYPES" or // This can probably be solved more nicely - put.getKey().toString() = "RMIConnectorServer.CREDENTIALS_FILTER_PATTERN" // This can probably be solved more nicely + put.getKey() + .(FieldAccess) + .getField() + .hasQualifiedName("javax.management.remote.rmi", "RMIConnectorServer", + ["CREDENTIAL_TYPES", "CREDENTIALS_FILTER_PATTERN", "SERIAL_FILTER_PATTERN"]) | put.getQualifier() = qualifier and put.getMethod().(MapMethod).getReceiverKeyType() instanceof TypeString and @@ -63,90 +68,21 @@ class MapToPutCredentialstypeConfiguration extends DataFlow2::Configuration { } } -/** - * Models flow from `new HashMap<>()` variable which is later used as environment during - * a JMX/RMI server initialization with `newJMXConnectorServer(...)` or `RMIConnectorServer(...)` - */ -class MapToRmiServerInitConfiguration extends DataFlow::Configuration { - MapToRmiServerInitConfiguration() { this = "MapToRmiServerInitConfiguration" } - - override predicate isSource(DataFlow::Node source) { - source.asExpr().(ClassInstanceExpr).getConstructedType() instanceof MapType - } - - override predicate isSink(DataFlow::Node sink) { - exists(ConstructorCall ccall | - sink.asExpr() = ccall.getArgument(1) and - isRmiOrJmxServerCreateConstructor(ccall.getConstructor()) - ) - or - exists(MethodAccess ma | - sink.asExpr() = ma.getArgument(1) and - isRmiOrJmxServerCreateMethod(ma.getMethod()) - ) - } -} - -/** Models if any JMX/RMI server are initialized with a null environment */ -class FlowServerInitializedWithNullEnv extends DataFlow::Configuration { - FlowServerInitializedWithNullEnv() { this = "FlowServerInitializedWithNullEnv" } - - override predicate isSource(DataFlow::Node source) { any() } - - override predicate isSink(DataFlow::Node sink) { - exists(ConstructorCall ccall | - sink.asExpr() = ccall and - isRmiOrJmxServerCreateConstructor(ccall.getConstructor()) and - ccall.getArgument(1) = alwaysNullExpr() - ) - or - exists(MethodAccess ma | - sink.asExpr() = ma and - isRmiOrJmxServerCreateMethod(ma.getMethod()) and - ma.getArgument(1) = alwaysNullExpr() - ) - } -} - -/** Holds if within the passed PathNode a "jmx.remote.rmi.server.credential.types" is set. */ -predicate mapFlowContainsCredentialtype(DataFlow::PathNode source) { - exists(MapToPutCredentialstypeConfiguration conf | conf.hasFlow(source.getNode(), _)) -} - /** Gets a string describing why the application is vulnerable, depending on if the vulnerability is present due to a) a null environment b) an insecurely set environment map */ -bindingset[source] -string getRmiResult(DataFlow::PathNode source) { +string getRmiResult(Expr e) { // We got a Map so we have a source and a sink node - if source.getNode().getType() instanceof MapType + if e instanceof NullLiteral then result = - "RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method." + "RMI/JMX server initialized with a null environment. Missing type restriction in RMI authentication method exposes the application to deserialization attacks." else - // The environment is not a map so we most likely have a "null" environment and therefore only a sink result = - "RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks." + "RMI/JMX server initialized with insecure environment $@, which never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method." } -/** Holds for any map flow paths with **no** jmx.remote.rmi.server.credential.types set */ -predicate hasVulnerableMapFlow(DataFlow::PathNode source, DataFlow::PathNode sink) { - exists(MapToRmiServerInitConfiguration dataflow | - dataflow.hasFlowPath(source, sink) and - not mapFlowContainsCredentialtype(source) - ) -} - -from - DataFlow::PathNode source, DataFlow::PathNode sink, - FlowServerInitializedWithNullEnv initNullDataflow +from Call c, Expr envArg where - // Check if server is created with null env - initNullDataflow.hasFlowPath(source, sink) - or - /* - * The map created by `new HashMap()` has to a) flow to the sink and - * b) there must not exist a (different) sink that would put `"jmx.remote.rmi.server.credential.types"` into `source`. - */ - - hasVulnerableMapFlow(source, sink) -select sink.getNode(), source, sink, getRmiResult(source), sink.getNode(), "here", source.getNode(), - "source environment 'Map'" + (isRmiOrJmxServerCreateConstructor(c.getCallee()) or isRmiOrJmxServerCreateMethod(c.getCallee())) and + envArg = c.getArgument(1) and + not any(SafeFlow conf).hasFlowToExpr(envArg) +select c, getRmiResult(envArg), envArg, envArg.toString() diff --git a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected index fc3cb78662e0..b2bd1f2ffb10 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected @@ -1,36 +1,4 @@ -edges -| ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:26:12:26:29 | this [post update] : RMIConnectorServer | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | -| InsecureRmiJmxEnvironmentConfiguration.java:25:31:25:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | -| InsecureRmiJmxEnvironmentConfiguration.java:33:31:33:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | -| InsecureRmiJmxEnvironmentConfiguration.java:40:31:40:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:44:59:44:61 | env | -| InsecureRmiJmxEnvironmentConfiguration.java:49:31:49:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:53:34:53:36 | env | -| InsecureRmiJmxEnvironmentConfiguration.java:58:31:58:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:62:59:62:61 | env | -| InsecureRmiJmxEnvironmentConfiguration.java:67:31:67:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:71:34:71:36 | env | -| InsecureRmiJmxEnvironmentConfiguration.java:76:31:76:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:80:59:80:61 | env | -| InsecureRmiJmxEnvironmentConfiguration.java:85:31:85:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:89:34:89:36 | env | -nodes -| ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:26:12:26:29 | this [post update] : RMIConnectorServer | semmle.label | this [post update] : RMIConnectorServer | -| InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | semmle.label | newJMXConnectorServer(...) | -| InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | semmle.label | new RMIConnectorServer(...) | -| InsecureRmiJmxEnvironmentConfiguration.java:25:31:25:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | -| InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | semmle.label | env | -| InsecureRmiJmxEnvironmentConfiguration.java:33:31:33:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | -| InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | semmle.label | env | -| InsecureRmiJmxEnvironmentConfiguration.java:40:31:40:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | -| InsecureRmiJmxEnvironmentConfiguration.java:44:59:44:61 | env | semmle.label | env | -| InsecureRmiJmxEnvironmentConfiguration.java:49:31:49:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | -| InsecureRmiJmxEnvironmentConfiguration.java:53:34:53:36 | env | semmle.label | env | -| InsecureRmiJmxEnvironmentConfiguration.java:58:31:58:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | -| InsecureRmiJmxEnvironmentConfiguration.java:62:59:62:61 | env | semmle.label | env | -| InsecureRmiJmxEnvironmentConfiguration.java:67:31:67:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | -| InsecureRmiJmxEnvironmentConfiguration.java:71:34:71:36 | env | semmle.label | env | -| InsecureRmiJmxEnvironmentConfiguration.java:76:31:76:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | -| InsecureRmiJmxEnvironmentConfiguration.java:80:59:80:61 | env | semmle.label | env | -| InsecureRmiJmxEnvironmentConfiguration.java:85:31:85:45 | new HashMap(...) : HashMap | semmle.label | new HashMap(...) : HashMap | -| InsecureRmiJmxEnvironmentConfiguration.java:89:34:89:36 | env | semmle.label | env | -#select -| InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | here | InsecureRmiJmxEnvironmentConfiguration.java:13:5:13:69 | newJMXConnectorServer(...) | source environment 'Map' | -| InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:26:12:26:29 | this [post update] : RMIConnectorServer | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | here | ../../../stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java:26:12:26:29 | this [post update] | source environment 'Map' | -| InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | RMI/JMX server initialized with 'null' environment $@. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | here | InsecureRmiJmxEnvironmentConfiguration.java:18:5:18:50 | new RMIConnectorServer(...) | source environment 'Map' | -| InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | InsecureRmiJmxEnvironmentConfiguration.java:25:31:25:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiJmxEnvironmentConfiguration.java:27:34:27:36 | env | here | InsecureRmiJmxEnvironmentConfiguration.java:25:31:25:45 | new HashMap(...) | source environment 'Map' | -| InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | InsecureRmiJmxEnvironmentConfiguration.java:33:31:33:45 | new HashMap(...) : HashMap | InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | RMI/JMX server initialized with insecure environment $@. The $@ never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiJmxEnvironmentConfiguration.java:35:59:35:61 | env | here | InsecureRmiJmxEnvironmentConfiguration.java:33:31:33:45 | new HashMap(...) | source environment 'Map' | +| InsecureRmiJmxEnvironmentConfiguration.java:12:5:12:69 | newJMXConnectorServer(...) | RMI/JMX server initialized with a null environment. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiJmxEnvironmentConfiguration.java:12:59:12:62 | null | null | +| InsecureRmiJmxEnvironmentConfiguration.java:17:5:17:50 | new RMIConnectorServer(...) | RMI/JMX server initialized with a null environment. Missing type restriction in RMI authentication method exposes the application to deserialization attacks. | InsecureRmiJmxEnvironmentConfiguration.java:17:34:17:37 | null | null | +| InsecureRmiJmxEnvironmentConfiguration.java:25:5:25:49 | new RMIConnectorServer(...) | RMI/JMX server initialized with insecure environment $@, which never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiJmxEnvironmentConfiguration.java:25:34:25:36 | env | env | +| InsecureRmiJmxEnvironmentConfiguration.java:33:5:33:68 | newJMXConnectorServer(...) | RMI/JMX server initialized with insecure environment $@, which never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method. | InsecureRmiJmxEnvironmentConfiguration.java:33:59:33:61 | env | env | diff --git a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java index 67ba5c89ea80..f1294847fcc0 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java +++ b/java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java @@ -1,6 +1,5 @@ import java.io.IOException; import javax.management.remote.JMXConnectorServerFactory; - import javax.management.remote.rmi.RMIConnectorServer; import java.util.HashMap; @@ -16,7 +15,6 @@ public void initInsecureJmxDueToNullEnv() throws IOException { public void initInsecureRmiDueToNullEnv() throws IOException { // Bad initializing env (arg1) with null new RMIConnectorServer(null, null, null, null); - } public void initInsecureRmiDueToMissingEnvKeyValue() throws IOException { @@ -71,7 +69,7 @@ public void secureeRmiConnectorServerConstants1() throws IOException { new RMIConnectorServer(null, env, null, null); } - public void secureeJmxConnectorServerConstants2() throws IOException { + public void secureJmxConnectorServerConstants2() throws IOException { // Good Map env = new HashMap<>(); env.put("jmx.remote.x.daemon", "true"); @@ -80,7 +78,7 @@ public void secureeJmxConnectorServerConstants2() throws IOException { JMXConnectorServerFactory.newJMXConnectorServer(null, env, null); } - public void secureeRmiConnectorServerConstants2() throws IOException { + public void secureRmiConnectorServerConstants2() throws IOException { // Good Map env = new HashMap<>(); env.put("jmx.remote.x.daemon", "true"); diff --git a/java/ql/test/experimental/query-tests/security/CWE-665/options b/java/ql/test/experimental/query-tests/security/CWE-665/options index 7fc584f11e39..4ecad5fd356d 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-665/options +++ b/java/ql/test/experimental/query-tests/security/CWE-665/options @@ -1 +1 @@ -//semmle-extractor-options: --javac-args -source 11 -target 11 -cp ${testdir}/../../../../experimental/stubs/javax-management-remote-rmi-0.0.1 +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/rmi-remote-0.0.0 diff --git a/java/ql/test/experimental/stubs/rmi-remote-0.0.0/README b/java/ql/test/experimental/stubs/rmi-remote-0.0.0/README new file mode 100644 index 000000000000..3ff60bac644a --- /dev/null +++ b/java/ql/test/experimental/stubs/rmi-remote-0.0.0/README @@ -0,0 +1 @@ +This is a workaround for a bug in which the extractor can't resolve type javax.management.remote.rmi.RMIConnectorServer even though it has been part of the JDK since Java 5 diff --git a/java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIConnection.java b/java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIConnection.java new file mode 100644 index 000000000000..68b41b3eb3a4 --- /dev/null +++ b/java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIConnection.java @@ -0,0 +1,6 @@ +package javax.management.remote.rmi; + +import java.rmi.Remote; +import java.io.Closeable; + +interface RMIConnection extends Closeable, Remote { } diff --git a/java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIConnectorServer.java b/java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIConnectorServer.java new file mode 100644 index 000000000000..d6f454c787c6 --- /dev/null +++ b/java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIConnectorServer.java @@ -0,0 +1,34 @@ +package javax.management.remote.rmi; + +import java.util.Map; + +import javax.management.remote.JMXConnectorServer; +import javax.management.remote.JMXConnector; +import javax.management.remote.JMXServiceURL; +import javax.management.remote.MBeanServerForwarder; +import javax.management.MBeanServer; + +// Note this is a partial stub sufficient to the needs of tests for CWE-665 +public class RMIConnectorServer extends JMXConnectorServer { + + public RMIConnectorServer(JMXServiceURL url, Map environment) { } + public RMIConnectorServer(JMXServiceURL url, Map environment, MBeanServer mbeanServer) { } + public RMIConnectorServer(JMXServiceURL url, Map environment, RMIServerImpl rmiServerImpl, MBeanServer mbeanServer) { } + + public static String CREDENTIAL_TYPES = ""; + public static String CREDENTIALS_FILTER_PATTERN = ""; + public static String JNDI_REBIND_ATTRIBUTE = ""; + public static String RMI_CLIENT_SOCKET_FACTORY_ATTRIBUTE = ""; + public static String RMI_SERVER_SOCKET_FACTORY_ATTRIBUTE = ""; + public static String SERIAL_FILTER_PATTERN = ""; + + public Map getAttributes() { return null; } + public JMXServiceURL getAddress() { return null; } + public String[] getConnectionIds() { return null; } + public boolean isActive() { return true; } + public void setMBeanServerForwarder(MBeanServerForwarder mbsf) { } + public void start() { } + public void stop() { } + public JMXConnector toJMXConnector(Map env) { return null; } + +} diff --git a/java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIServer.java b/java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIServer.java new file mode 100644 index 000000000000..d08429b1dd67 --- /dev/null +++ b/java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIServer.java @@ -0,0 +1,3 @@ +package javax.management.remote.rmi; + +interface RMIServer { } diff --git a/java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIServerImpl.java b/java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIServerImpl.java new file mode 100644 index 000000000000..91466e087b30 --- /dev/null +++ b/java/ql/test/experimental/stubs/rmi-remote-0.0.0/javax/management/remote/rmi/RMIServerImpl.java @@ -0,0 +1,12 @@ +package javax.management.remote.rmi; + +import java.io.Closeable; +import java.rmi.Remote; + +public class RMIServerImpl implements Closeable, RMIServer { + + public void close() { } + public String getVersion() { return null; } + public RMIConnection newClient(Object credentials) { return null; } + +} From d0478eac95dbd1486a3009e79315016f3c4a8c17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Fri, 25 Jun 2021 09:45:46 +0200 Subject: [PATCH 25/30] XML validation and spelling/ordering changes * XML validation and summary changes in qhelp file ; * Encode entities within snippet * Updated minor descriptions and examples * Implemented spelling review --- ...secureRmiJmxEnvironmentConfiguration.qhelp | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp index f20b55d1a1a9..55d5e3c63e27 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp @@ -2,34 +2,42 @@ "-//Semmle//qhelp//EN" "qhelp.dtd"> + -

    An improperly set environment variable during the creation of an RMI or JMX server can lead -to an unauthenticated remote code execution vulnerability. This is because the -RMI/JMX server environment allows attackers to supply arbitrary objects to the authentication -method, resulting in the attempted deserialization of an attacker-controlled object. +

    For special use cases some applications may implement a custom service which handles JMX-RMI connections.

    + +

    When creating such a custom service, a developer should pass a certain environment configuration to the JMX-RMI server initalisation, +as otherwise the JMX-RMI service is susceptible to an unsafe deserialization vulnerability.

    + +

    This is because the JMX-RMI service allows attackers to supply arbitrary objects to the service authentication +method, resulting in the attempted deserialization of an attacker-controlled object. +In the worst case scenario this could allow an attacker to achieve remote code execution within the context of the application server.

    + +

    By setting the appropriate environment, the deserialization can be controlled via a deserialization filter.

    +     -

    During the creation/initialization of an RMI or JMX server an environment should be supplied that sets a deserialization filter. -Ideally this filter only allows the deserialization of java.lang.String. +

    During the creation of a custom JMX-RMI service an environment should be supplied that sets a deserialization filter. +Ideally this filter should be as restrictive as possible, for example to only allow the deserialization of java.lang.String.

    -The filter can be configured by setting the key jmx.remote.rmi.server.credentials.filter.pattern (given by the constant RMIConnectorServer.CREDENTIALS_FILTER_PATTERN). -The filter should (ideally) only allow java.lang.String and disallow all other classes for deserialization: ("java.lang.String;!*"). +

    The filter can be configured by setting the key jmx.remote.rmi.server.credentials.filter.pattern (given by the constant RMIConnectorServer.CREDENTIALS_FILTER_PATTERN). +The filter should (ideally) only allow java.lang.String and disallow all other classes for deserialization: ("java.lang.String;!*").

    -The key-value pair can be set as following: +

    The key-value pair can be set as following:

    String stringsOnlyFilter = "java.lang.String;!*"; // Deny everything but java.lang.String -Map env = new HashMap; +Map<String, Object> env = new HashMap<String, Object>; env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, stringsOnlyFilter); -For applications using Java 9 or below: +

    For applications using Java 6u113 to 9:

    // This is deprecated in Java 10+ ! -Map env = new HashMap; +Map<String, Object> env = new HashMap<String, Object>; env.put ( "jmx.remote.rmi.server.credential.types", new String[]{ @@ -39,20 +47,20 @@ env.put ( ); -Please note that the authentication implementation is vulnerable by default. -For this reason an initialization with a null environment is also vulnerable. +

    Please note that the JMX-RMI service is vulnerable in the default configuration. +For this reason an initialization with a null environment is also vulnerable.

     -

    The following examples show how an RMI or JMX server can be initialized securely. +

    The following examples show how an JMX-RMI service can be initialized securely.

    -

    The first example shows how an RMI server can be initialized with a secure environment.

    +

    The first example shows how an JMX server is initialized securely with the JMXConnectorServerFactory.newJMXConnectorServer() call.

    - + -

    The second example shows how the environment for a JMX server can be initialized securely.

    +

    The second example shows how a JMX Server is initialized securely if the RMIConnectorServer class is used.

    - +     @@ -63,4 +71,5 @@ For this reason an initialization with a null environment is also v
  • Java 10 API specification for RMIConnectorServer.CREDENTIALS_FILTER_PATTERN
  • The Java API specification for RMIConnectorServer.CREDENTIAL_TYPES. Please note that this field is deprecated since Java 10.
    • +     From 5aeeb3a801f15e40a1b79d7f63bc62c44a10c103 Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Fri, 25 Jun 2021 15:37:47 +0200 Subject: [PATCH 26/30] Fixed and validated qhelp --- ...rverFactoryEnvironmentInitialisation.java} | 0 ...nectorServerEnvironmentInitalisation.java} | 0 ...secureRmiJmxEnvironmentConfiguration.qhelp | 23 ++++--------------- .../CWE/CWE-665/example_filter_java_10.java | 4 ++++ .../CWE/CWE-665/example_filter_java_9.java | 9 ++++++++ 5 files changed, 17 insertions(+), 19 deletions(-) rename java/ql/src/experimental/Security/CWE/CWE-665/{CorrectJmxEnvironmentInitialisation.java => CorrectJMXConnectorServerFactoryEnvironmentInitialisation.java} (100%) rename java/ql/src/experimental/Security/CWE/CWE-665/{CorrectRmiEnvironmentInitialisation.java => CorrectRMIConnectorServerEnvironmentInitalisation.java} (100%) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-665/example_filter_java_10.java create mode 100644 java/ql/src/experimental/Security/CWE/CWE-665/example_filter_java_9.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectJMXConnectorServerFactoryEnvironmentInitialisation.java similarity index 100% rename from java/ql/src/experimental/Security/CWE/CWE-665/CorrectJmxEnvironmentInitialisation.java rename to java/ql/src/experimental/Security/CWE/CWE-665/CorrectJMXConnectorServerFactoryEnvironmentInitialisation.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java b/java/ql/src/experimental/Security/CWE/CWE-665/CorrectRMIConnectorServerEnvironmentInitalisation.java similarity index 100% rename from java/ql/src/experimental/Security/CWE/CWE-665/CorrectRmiEnvironmentInitialisation.java rename to java/ql/src/experimental/Security/CWE/CWE-665/CorrectRMIConnectorServerEnvironmentInitalisation.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp index 55d5e3c63e27..c74d5a9d4b4d 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp @@ -26,26 +26,11 @@ The filter should (ideally) only allow java.lang.String and disallow all other c

    The key-value pair can be set as following:

    - -String stringsOnlyFilter = "java.lang.String;!*"; // Deny everything but java.lang.String - -Map<String, Object> env = new HashMap<String, Object>; -env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, stringsOnlyFilter); - +

    For applications using Java 6u113 to 9:

    - -// This is deprecated in Java 10+ ! -Map<String, Object> env = new HashMap<String, Object>; -env.put ( - "jmx.remote.rmi.server.credential.types", - new String[]{ - String[].class.getName(), - String.class.getName() - } - ); - +

    Please note that the JMX-RMI service is vulnerable in the default configuration. For this reason an initialization with a null environment is also vulnerable.

    @@ -56,11 +41,11 @@ For this reason an initialization with a null environment is also v

    The first example shows how an JMX server is initialized securely with the JMXConnectorServerFactory.newJMXConnectorServer() call.

    - +

    The second example shows how a JMX Server is initialized securely if the RMIConnectorServer class is used.

    - + diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/example_filter_java_10.java b/java/ql/src/experimental/Security/CWE/CWE-665/example_filter_java_10.java new file mode 100644 index 000000000000..0ffc7f282223 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-665/example_filter_java_10.java @@ -0,0 +1,4 @@ +String stringsOnlyFilter = "java.lang.String;!*"; // Deny everything but java.lang.String + +Map env = new HashMap; +env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, stringsOnlyFilter); \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/example_filter_java_9.java b/java/ql/src/experimental/Security/CWE/CWE-665/example_filter_java_9.java new file mode 100644 index 000000000000..4001f63bb815 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-665/example_filter_java_9.java @@ -0,0 +1,9 @@ +// This is deprecated in Java 10+ ! +Map; env = new HashMap; +env.put ( + "jmx.remote.rmi.server.credential.types", + new String[]{ + String[].class.getName(), + String.class.getName() + } + ); \ No newline at end of file From 328b69f46cb2486a1e74101030cbffb039e5f4b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Fri, 25 Jun 2021 16:10:20 +0200 Subject: [PATCH 27/30] Update java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index eab6f1f77179..894304955a91 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -52,9 +52,10 @@ class SafeFlow extends DataFlow::Configuration { private predicate putsCredentialtypesKey(Expr qualifier) { exists(MapPutCall put | put.getKey().(CompileTimeConstantExpr).getStringValue() = - "jmx.remote.rmi.server.credential.types" or - put.getKey().(CompileTimeConstantExpr).getStringValue() = - "jmx.remote.rmi.server.credentials.filter.pattern" or + ["jmx.remote.rmi.server.credential.types", + "jmx.remote.rmi.server.credentials.filter.pattern"] + + or put.getKey() .(FieldAccess) .getField() From 72ef4983dbf475c0d395be40edc4080e36b3fb78 Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Fri, 25 Jun 2021 16:11:37 +0200 Subject: [PATCH 28/30] Fixed wrong match for symbolic constant --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index eab6f1f77179..cf8e65c3bfb1 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -59,7 +59,7 @@ class SafeFlow extends DataFlow::Configuration { .(FieldAccess) .getField() .hasQualifiedName("javax.management.remote.rmi", "RMIConnectorServer", - ["CREDENTIAL_TYPES", "CREDENTIALS_FILTER_PATTERN", "SERIAL_FILTER_PATTERN"]) + ["CREDENTIAL_TYPES", "CREDENTIALS_FILTER_PATTERN"]) | put.getQualifier() = qualifier and put.getMethod().(MapMethod).getReceiverKeyType() instanceof TypeString and From 8daa398af62eb5e3e87c5d476886f798f033e30f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20M=C3=BCller?= Date: Fri, 25 Jun 2021 16:12:37 +0200 Subject: [PATCH 29/30] Update InsecureRmiJmxEnvironmentConfiguration.ql --- .../CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index 894304955a91..e361af39cf44 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -60,7 +60,7 @@ class SafeFlow extends DataFlow::Configuration { .(FieldAccess) .getField() .hasQualifiedName("javax.management.remote.rmi", "RMIConnectorServer", - ["CREDENTIAL_TYPES", "CREDENTIALS_FILTER_PATTERN", "SERIAL_FILTER_PATTERN"]) + ["CREDENTIAL_TYPES", "CREDENTIALS_FILTER_PATTERN"]) | put.getQualifier() = qualifier and put.getMethod().(MapMethod).getReceiverKeyType() instanceof TypeString and From e5fa5325b5e25ea8f6bf25ee34e2a30819430750 Mon Sep 17 00:00:00 2001 From: Timo Mueller Date: Fri, 25 Jun 2021 22:31:29 +0200 Subject: [PATCH 30/30] Auto formatting .ql file --- .../CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql index e361af39cf44..9733ccf7b559 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql @@ -52,10 +52,11 @@ class SafeFlow extends DataFlow::Configuration { private predicate putsCredentialtypesKey(Expr qualifier) { exists(MapPutCall put | put.getKey().(CompileTimeConstantExpr).getStringValue() = - ["jmx.remote.rmi.server.credential.types", - "jmx.remote.rmi.server.credentials.filter.pattern"] - - or + [ + "jmx.remote.rmi.server.credential.types", + "jmx.remote.rmi.server.credentials.filter.pattern" + ] + or put.getKey() .(FieldAccess) .getField()