Java: Add query - insecure environment configuration during JMX/RMI server init

java/ql/src/experimental/Security/CWE/CWE-665/CorrectJMXConnectorServerFactoryEnvironmentInitialisation.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<String, Object> env = new HashMap<String, Object>();
+        // For Java 10+
+        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",
+                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();
+    }
+}

java/ql/src/experimental/Security/CWE/CWE-665/CorrectRMIConnectorServerEnvironmentInitalisation.java

@@ -0,0 +1,32 @@
+public class CorrectRmiInitialisation {
+    public void initAndStartRmiServer(int port, String hostname, boolean local) {
+        MBeanServerForwarder authzProxy = null;
+
+        env.put("jmx.remote.x.daemon", "true");
+
+        /* Restrict the login function to String Objects only (see CVE-2016-3427) */
+        Map<String, Object> env = new HashMap<String, Object>();
+        // For Java 10+
+        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",
+                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();
+
+    }
+}

java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qhelp

@@ -0,0 +1,60 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>For special use cases some applications may implement a custom service which handles JMX-RMI connections.</p>
+
+<p>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.</p>
+
+<p>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.</p>
+
+<p>By setting the appropriate environment, the deserialization can be controlled via a deserialization filter.</p>
+
+</overview>
+
+<recommendation>
+<p>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 <code>java.lang.String</code>.</p>
+
+<p>The filter can be configured by setting the key <code>jmx.remote.rmi.server.credentials.filter.pattern</code> (given by the constant <code>RMIConnectorServer.CREDENTIALS_FILTER_PATTERN</code>).
+The filter should (ideally) only allow java.lang.String and disallow all other classes for deserialization: (<code>"java.lang.String;!*"</code>).</p>
+
+<p>The key-value pair can be set as following:</p>
+
+<sample src="example_filter_java_10.java" />
+
+<p>For applications using Java 6u113 to 9:</p>
+
+<sample src="example_filter_java_9.java" />
+
+<p>Please note that the JMX-RMI service is vulnerable in the default configuration.
+For this reason an initialization with a <code>null</code> environment is also vulnerable.</p>
+</recommendation>
+
+<example>
+<p>The following examples show how an JMX-RMI service can be initialized securely.</p>
+
+<p>The first example shows how an JMX server is initialized securely with the <code>JMXConnectorServerFactory.newJMXConnectorServer()</code> call.</p>
+
+<sample src="CorrectJMXConnectorServerFactoryEnvironmentInitialisation.java" />
+
+<p>The second example shows how a JMX Server is initialized securely if the <code>RMIConnectorServer</code> class is used.</p>
+
+<sample src="CorrectRMIConnectorServerEnvironmentInitalisation.java" />
+
+</example>
+
+<references>
+<li>Deserialization of arbitrary objects could lead to remote code execution as described following: <a href="https://owasp.org/www-community/vulnerabilities/Deserialization_of_untrusted_data">OWASP Deserialization of untrusted data</a>.</li>
+<li>Issue discovered in Tomcat (CVE-2016-8735): <a href="https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-8735">OWASP ESAPI</a>.</li>
+<li><a href="https://www.oracle.com/java/technologies/javase/8u91-relnotes.html#bugfixes-8u91">Oracle release notes</a>: New attribute for JMX RMI JRMP servers.</li>
+<li>Java 10 API specification for <a href="https://docs.oracle.com/javase/10/docs/api/javax/management/remote/rmi/RMIConnectorServer.html#CREDENTIALS_FILTER_PATTERN">RMIConnectorServer.CREDENTIALS_FILTER_PATTERN</a></li>
+<li>The Java API specification for <a href="https://docs.oracle.com/javase/10/docs/api/javax/management/remote/rmi/RMIConnectorServer.html#CREDENTIAL_TYPES">RMIConnectorServer.CREDENTIAL_TYPES</a>. Please note that this field is deprecated since Java 10.</li>
+</references>
+
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql

@@ -0,0 +1,90 @@
+/**
+ * @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 problem
+ * @problem.severity error
+ * @tags security
+ *       external/cwe/cwe-665
+ * @precision high
+ * @id java/insecure-rmi-jmx-server-initialization
+ */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.Maps
+
+/** Holds if `constructor` instantiates an RMI or JMX server. */
+predicate isRmiOrJmxServerCreateConstructor(Constructor constructor) {
+  constructor
+      .getDeclaringType()
+      .hasQualifiedName("javax.management.remote.rmi", "RMIConnectorServer")
+}
+
+/** Holds if `method` creates an RMI or JMX server. */
+predicate isRmiOrJmxServerCreateMethod(Method method) {
+  method.getName() = "newJMXConnectorServer" and
+  method.getDeclaringType().hasQualifiedName("javax.management.remote", "JMXConnectorServerFactory")
+}
+
+/**
+ * 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 SafeFlow extends DataFlow::Configuration {
+  SafeFlow() { this = "MapToPutCredentialstypeConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { putsCredentialtypesKey(source.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
+   * into the map.
+   */
+  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
+      put.getKey()
+          .(FieldAccess)
+          .getField()
+          .hasQualifiedName("javax.management.remote.rmi", "RMIConnectorServer",
+            ["CREDENTIAL_TYPES", "CREDENTIALS_FILTER_PATTERN"])
+    |
+      put.getQualifier() = qualifier and
+      put.getMethod().(MapMethod).getReceiverKeyType() instanceof TypeString and
+      put.getMethod().(MapMethod).getReceiverValueType() instanceof TypeObject
+    )
+  }
+}
+
+/** 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 */
+string getRmiResult(Expr e) {
+  // We got a Map so we have a source and a sink node
+  if e instanceof NullLiteral
+  then
+    result =
+      "RMI/JMX server initialized with a null environment. Missing type restriction in RMI authentication method exposes the application to deserialization attacks."
+  else
+    result =
+      "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."
+}
+
+from Call c, Expr envArg
+where
+  (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()

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<String, Object> env = new HashMap<String, Object>;
+env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, stringsOnlyFilter);

java/ql/src/experimental/Security/CWE/CWE-665/example_filter_java_9.java

@@ -0,0 +1,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()
+   }
+ );

java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.expected

@@ -0,0 +1,4 @@
+| 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 |

java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.java

@@ -0,0 +1,89 @@
+import java.io.IOException;
+import javax.management.remote.JMXConnectorServerFactory;
+import javax.management.remote.rmi.RMIConnectorServer;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class InsecureRmiJmxEnvironmentConfiguration {
+
+  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);
+  }
+
+  public void initInsecureRmiDueToMissingEnvKeyValue() throws IOException {
+    // Bad initializing env (arg1) with missing
+    // "jmx.remote.rmi.server.credential.types"
+    Map<String, Object> env = new HashMap<>();
+    env.put("jmx.remote.x.daemon", "true");
+    new RMIConnectorServer(null, env, null, null);
+  }
+
+  public void initInsecureJmxDueToMissingEnvKeyValue() throws IOException {
+    // Bad initializing env (arg1) with missing
+    // "jmx.remote.rmi.server.credential.types"
+    Map<String, Object> env = new HashMap<>();
+    env.put("jmx.remote.x.daemon", "true");
+    JMXConnectorServerFactory.newJMXConnectorServer(null, env, null);
+  }
+
+  public void secureJmxConnnectorServer() throws IOException {
+    // Good
+    Map<String, Object> 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<String, Object> 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);
+  }
+
+  public void secureeJmxConnectorServerConstants1() throws IOException {
+    // Good
+    Map<String, Object> env = new HashMap<>();
+    env.put("jmx.remote.x.daemon", "true");
+    env.put(RMIConnectorServer.CREDENTIALS_FILTER_PATTERN, "java.lang.String;!*"); // Deny everything but
+                                                                                   // java.lang.String
+    JMXConnectorServerFactory.newJMXConnectorServer(null, env, null);
+  }
+
+  public void secureeRmiConnectorServerConstants1() throws IOException {
+    // Good
+    Map<String, Object> env = new HashMap<>();
+    env.put("jmx.remote.x.daemon", "true");
+    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 secureJmxConnectorServerConstants2() throws IOException {
+    // Good
+    Map<String, Object> 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 secureRmiConnectorServerConstants2() throws IOException {
+    // Good
+    Map<String, Object> 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);
+  }
+}

java/ql/test/experimental/query-tests/security/CWE-665/InsecureRmiJmxEnvironmentConfiguration.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-665/InsecureRmiJmxEnvironmentConfiguration.ql

java/ql/test/experimental/query-tests/security/CWE-665/options

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

java/ql/test/experimental/stubs/javax-management-remote-rmi-0.0.1/javax/management/remote/rmi/RMIConnectorServer.java

@@ -0,0 +1,30 @@
+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 static final String CREDENTIALS_FILTER_PATTERN = "jmx.remote.rmi.server.credentials.filter.pattern";
+
+    public RMIConnectorServer(JMXServiceURL url, Map<String, ?> environment) throws IOException {
+        // stub;
+    }
+
+    public RMIConnectorServer(JMXServiceURL url, Map<String, ?> environment, MBeanServer mbeanServer)
+            throws IOException {
+        // stub;
+    }
+
+    public RMIConnectorServer(JMXServiceURL url, Map<String, ?> environment, RMIServerImpl rmiServerImpl,
+            MBeanServer mbeanServer) throws IOException {
+        // stub;
+    }
+}

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<String, ?> env) {
+        // stub;
+    }
+
+}

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

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 { }