Java: Query for detecting unsafe deserialization with Spring exporters

java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp

@@ -0,0 +1,81 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Spring Framework provides an abstract base class <code>RemoteInvocationSerializingExporter</code>
+for defining remote service exporters.
+A Spring exporter, which is based on this class, deserializes incoming data using <code>ObjectInputStream</code>.
+Deserializing untrusted data is easily exploitable and in many cases allows an attacker
+to execute arbitrary code.
+</p>
+<p>
+Spring Framework also provides two classes that extend <code>RemoteInvocationSerializingExporter</code>:
+<li>
+<code>HttpInvokerServiceExporter</code>
+</li>
+<li>
+<code>SimpleHttpInvokerServiceExporter</code>
+</li>
+</p>
+<p>
+These classes export specified beans as HTTP endpoints that deserialize data from an HTTP request
+using unsafe <code>ObjectInputStream</code>. If a remote attacker can reach such endpoints,
+it results in remote code execution in the worst case.
+</p>
+<p>
+CVE-2016-1000027 has been assigned to this issue in Spring Framework. There is no fix for that.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Avoid using <code>HttpInvokerServiceExporter</code>, <code>SimpleHttpInvokerServiceExporter</code>
+and any other exporter that is based on <code>RemoteInvocationSerializingExporter</code>.
+Instead, use other message formats for API endpoints (for example, JSON),
+but make sure that the underlying deserialization mechanism is properly configured
+so that deserialization attacks are not possible. If the vulnerable exporters can not be replaced,
+consider using global deserialization filters introduced by JEP 290.
+In general, avoid deserialization of untrusted data.
+</p>
+</recommendation>
+
+<example>
+<p>
+The following example defines a vulnerable HTTP endpoint:
+</p>
+<sample src="UnsafeHttpInvokerEndpoint.java" />
+</example>
+
+<references>
+<li>
+OWASP:
+<a href="https://www.owasp.org/index.php/Deserialization_of_untrusted_data">Deserialization of untrusted data</a>.
+</li>
+<li>
+Spring Framework API documentation:
+<a href="https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/remoting/rmi/RemoteInvocationSerializingExporter.html">RemoteInvocationSerializingExporter class</a>
+</li>
+<li>
+Spring Framework API documentation:
+<a href="https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/remoting/httpinvoker/HttpInvokerServiceExporter.html">HttpInvokerServiceExporter class</a>
+</li>
+<li>
+National Vulnerability Database:
+<a href="https://nvd.nist.gov/vuln/detail/CVE-2016-1000027">CVE-2016-1000027</a>
+</li>
+<li>
+Tenable Research Advisory:
+<a href="https://www.tenable.com/security/research/tra-2016-20">[R2] Pivotal Spring Framework HttpInvokerServiceExporter readRemoteInvocation Method Untrusted Java Deserialization</a>
+</li>
+<li>
+Spring Framework bug tracker:
+<a href="https://github.com/spring-projects/spring-framework/issues/24434">Sonatype vulnerability CVE-2016-1000027 in Spring-web project</a>  
+</li>
+<li>
+OpenJDK:
+<a href="https://openjdk.java.net/jeps/290">JEP 290: Filter Incoming Serialization Data</a>
+</li>
+</references>
+
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql

@@ -0,0 +1,55 @@
+/**
+ * @name Unsafe deserialization with spring's remote service exporters.
+ * @description Creating a bean based on RemoteInvocationSerializingExporter
+ *              may lead to arbitrary code execution.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @id java/spring-exporter-unsafe-deserialization
+ * @tags security
+ *       external/cwe/cwe-502
+ */
+
+import java
+
+/**
+ * Holds if `method` initializes a bean.
+ */
+private predicate createsBean(Method method) {
+  method.hasAnnotation("org.springframework.context.annotation", "Bean")
+}
+
+/**
+ * Holds if `type` is `RemoteInvocationSerializingExporter`.
+ */
+private predicate isRemoteInvocationSerializingExporter(RefType type) {
+  type.hasQualifiedName("org.springframework.remoting.rmi", "RemoteInvocationSerializingExporter")
+}
+
+/**
+ * Holds if `method` returns an object that extends `RemoteInvocationSerializingExporter`.
+ */
+private predicate returnsRemoteInvocationSerializingExporter(Method method) {
+  isRemoteInvocationSerializingExporter(method.getReturnType().(RefType).getASupertype*())
+}
+
+/**
+ * Holds if `method` belongs to a Spring configuration.
+ */
+private predicate isInConfiguration(Method method) {
+  method.getDeclaringType().hasAnnotation("org.springframework.context.annotation", "Configuration")
+}
+
+/**
+ * Holds if `method` initializes a bean that is based on `RemoteInvocationSerializingExporter`.
+ */
+private predicate createsRemoteInvocationSerializingExporterBean(Method method) {
+  isInConfiguration(method) and
+  createsBean(method) and
+  returnsRemoteInvocationSerializingExporter(method)
+}
+
+from Method method
+where createsRemoteInvocationSerializingExporterBean(method)
+select method,
+  "Unasafe deserialization in a remote service exporter in '" + method.getName() + "' method"

java/ql/src/experimental/Security/CWE/CWE-502/UnsafeHttpInvokerEndpoint.java

@@ -0,0 +1,24 @@
+@Configuration
+public class Server {
+
+    @Bean(name = "/account")
+    HttpInvokerServiceExporter accountService() {
+        HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter();
+        exporter.setService(new AccountServiceImpl());
+        exporter.setServiceInterface(AccountService.class);
+        return exporter;
+    }
+
+}
+
+class AccountServiceImpl implements AccountService {
+
+    @Override
+    public String echo(String data) {
+        return data;
+    }
+}
+
+interface AccountService {
+    String echo(String data);
+}

java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.expected

@@ -0,0 +1,2 @@
+| SpringHttpInvokerUnsafeDeserialization.java:10:32:10:63 | unsafeHttpInvokerServiceExporter | Unasafe deserialization in a remote service exporter in 'unsafeHttpInvokerServiceExporter' method |
+| SpringHttpInvokerUnsafeDeserialization.java:18:41:18:88 | unsafeCustomeRemoteInvocationSerializingExporter | Unasafe deserialization in a remote service exporter in 'unsafeCustomeRemoteInvocationSerializingExporter' method |

java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.java

@@ -0,0 +1,53 @@
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.remoting.httpinvoker.HttpInvokerServiceExporter;
+import org.springframework.remoting.rmi.RemoteInvocationSerializingExporter;
+
+@Configuration
+public class SpringHttpInvokerUnsafeDeserialization {
+
+    @Bean(name = "/unsafeHttpInvokerServiceExporter")
+    HttpInvokerServiceExporter unsafeHttpInvokerServiceExporter() {
+        HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter();
+        exporter.setService(new AccountServiceImpl());
+        exporter.setServiceInterface(AccountService.class);
+        return exporter;
+    }
+
+    @Bean(name = "/unsafeCustomeRemoteInvocationSerializingExporter")
+    RemoteInvocationSerializingExporter unsafeCustomeRemoteInvocationSerializingExporter() {
+        return new CustomeRemoteInvocationSerializingExporter();
+    }
+
+    HttpInvokerServiceExporter notABean() {
+        HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter();
+        exporter.setService(new AccountServiceImpl());
+        exporter.setServiceInterface(AccountService.class);
+        return exporter;
+    }
+}
+
+class CustomeRemoteInvocationSerializingExporter extends RemoteInvocationSerializingExporter {}
+
+class NotAConfiguration {
+
+    @Bean(name = "/notAnEndpoint")
+    HttpInvokerServiceExporter notAnEndpoint() {
+        HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter();
+        exporter.setService(new AccountServiceImpl());
+        exporter.setServiceInterface(AccountService.class);
+        return exporter;
+    }
+}
+
+class AccountServiceImpl implements AccountService {
+
+    @Override
+    public String echo(String data) {
+        return data;
+    }
+}
+
+interface AccountService {
+    String echo(String data);
+}

java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql

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

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

java/ql/test/stubs/springframework-5.2.3/org/springframework/context/annotation/Bean.java

@@ -0,0 +1,10 @@
+package org.springframework.context.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target({ElementType.METHOD, ElementType.ANNOTATION_TYPE})
+public @interface Bean {
+
+	String[] name() default {};
+}

java/ql/test/stubs/springframework-5.2.3/org/springframework/context/annotation/Configuration.java

@@ -0,0 +1,7 @@
+package org.springframework.context.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target(ElementType.TYPE)
+public @interface Configuration {}

java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/httpinvoker/HttpInvokerServiceExporter.java

@@ -0,0 +1,8 @@
+package org.springframework.remoting.httpinvoker;
+
+public class HttpInvokerServiceExporter extends org.springframework.remoting.rmi.RemoteInvocationSerializingExporter {
+
+    public void setService(Object service) {}
+
+    public void setServiceInterface(Class clazz) {}
+}

java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RemoteInvocationSerializingExporter.java

@@ -0,0 +1,3 @@
+package org.springframework.remoting.rmi;
+
+public abstract class RemoteInvocationSerializingExporter {}