Merge pull request #5260 from artem-smotrakov/spring-http-invoker

Java: Query for detecting unsafe deserialization with Spring exporters

java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.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/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.xml

@@ -0,0 +1,4 @@
+<bean name="/account" class="org.springframework.remoting.httpinvoker.HttpInvokerServiceExporter">
+    <property name="service" ref="accountService"/>
+    <property name="serviceInterface" value="AccountService"/>
+</bean>

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

@@ -0,0 +1,8 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+  <include src="UnsafeSpringExporterQuery.inc.qhelp" />
+  <include src="UnsafeSpringExporterInConfigurationClassExample.inc.qhelp" />
+  <include src="UnsafeSpringExporterReferences.inc.qhelp" />
+</qhelp>

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

@@ -0,0 +1,58 @@
+/**
+ * @name Unsafe deserialization with Spring's remote service exporters.
+ * @description A Spring bean, which is based on RemoteInvocationSerializingExporter,
+ *              initializes an endpoint that uses ObjectInputStream to deserialize
+ *              incoming data. In the worst case, that may lead to remote code execution.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @id java/unsafe-deserialization-spring-exporter-in-configuration-class
+ * @tags security
+ *       external/cwe/cwe-502
+ */
+
+import java
+import UnsafeSpringExporterLib
+
+/**
+ * Holds if `type` is a Spring configuration that declares beans.
+ */
+private predicate isConfiguration(RefType type) {
+  type.hasAnnotation("org.springframework.context.annotation", "Configuration") or
+  isConfigurationAnnotation(type.getAnAnnotation())
+}
+
+/**
+ * Holds if `annotation` is a Java annotations that declares a Spring configuration.
+ */
+private predicate isConfigurationAnnotation(Annotation annotation) {
+  isConfiguration(annotation.getType()) or
+  isConfigurationAnnotation(annotation.getType().getAnAnnotation())
+}
+
+/**
+ * A method that initializes a unsafe bean based on `RemoteInvocationSerializingExporter`.
+ */
+private class UnsafeBeanInitMethod extends Method {
+  string identifier;
+
+  UnsafeBeanInitMethod() {
+    isRemoteInvocationSerializingExporter(this.getReturnType()) and
+    isConfiguration(this.getDeclaringType()) and
+    exists(Annotation a | this.getAnAnnotation() = a |
+      a.getType().hasQualifiedName("org.springframework.context.annotation", "Bean") and
+      if a.getValue("name") instanceof StringLiteral
+      then identifier = a.getValue("name").(StringLiteral).getRepresentedString()
+      else identifier = this.getName()
+    )
+  }
+
+  /**
+   * Gets this bean's name if given by the `Bean` annotation, or this method's identifier otherwise.
+   */
+  string getBeanIdentifier() { result = identifier }
+}
+
+from UnsafeBeanInitMethod method
+select method,
+  "Unsafe deserialization in a Spring exporter bean '" + method.getBeanIdentifier() + "'"

java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClassExample.inc.qhelp

@@ -0,0 +1,14 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<example>
+<p>
+The following example shows how a vulnerable HTTP endpoint can be defined
+using <code>HttpInvokerServiceExporter</code> and Spring annotations:
+</p>
+<sample src="SpringExporterUnsafeDeserialization.java" />
+</example>
+
+</qhelp>

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

@@ -0,0 +1,8 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+  <include src="UnsafeSpringExporterQuery.inc.qhelp" />
+  <include src="UnsafeSpringExporterInXMLConfigurationExample.inc.qhelp" />
+  <include src="UnsafeSpringExporterReferences.inc.qhelp" />
+</qhelp>

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

@@ -0,0 +1,20 @@
+/**
+ * @name Unsafe deserialization with Spring's remote service exporters.
+ * @description A Spring bean, which is based on RemoteInvocationSerializingExporter,
+ *              initializes an endpoint that uses ObjectInputStream to deserialize
+ *              incoming data. In the worst case, that may lead to remote code execution.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @id java/unsafe-deserialization-spring-exporter-in-xml-configuration
+ * @tags security
+ *       external/cwe/cwe-502
+ */
+
+import java
+import semmle.code.java.frameworks.spring.SpringBean
+import UnsafeSpringExporterLib
+
+from SpringBean bean
+where isRemoteInvocationSerializingExporter(bean.getClass())
+select bean, "Unsafe deserialization in a Spring exporter bean '" + bean.getBeanIdentifier() + "'"

java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfigurationExample.inc.qhelp

@@ -0,0 +1,13 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<example>
+<p>
+The following examples shows how a vulnerable HTTP endpoint can be defined in a Spring XML config:
+</p>
+<sample src="SpringExporterUnsafeDeserialization.xml" />
+</example>
+
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterLib.qll

@@ -0,0 +1,9 @@
+import java
+
+/**
+ * Holds if `type` is `RemoteInvocationSerializingExporter`.
+ */
+predicate isRemoteInvocationSerializingExporter(RefType type) {
+  type.getASupertype*()
+      .hasQualifiedName("org.springframework.remoting.rmi", "RemoteInvocationSerializingExporter")
+}

java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterQuery.inc.qhelp

@@ -0,0 +1,41 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+The Spring Framework provides an abstract base class <code>RemoteInvocationSerializingExporter</code>
+for creating 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>
+The Spring Framework also provides <code>HttpInvokerServiceExporter</code>
+and <code>SimpleHttpInvokerServiceExporter</code> classes
+that extend <code>RemoteInvocationSerializingExporter</code>.
+</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 the Spring Framework.
+It is regarded as a design limitation, and can be mitigated but not fixed outright.
+</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 in JEP 290.
+</p>
+</recommendation>
+
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterReferences.inc.qhelp

@@ -0,0 +1,37 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<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/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.java

@@ -0,0 +1,79 @@
+import org.springframework.boot.SpringBootConfiguration;
+import org.springframework.boot.autoconfigure.SpringBootApplication;
+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 SpringExporterUnsafeDeserialization {
+
+    @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;
+    }
+}
+
+@SpringBootApplication
+class SpringBootTestApplication {
+
+    @Bean(name = "/unsafeHttpInvokerServiceExporter")
+    HttpInvokerServiceExporter unsafeHttpInvokerServiceExporter() {
+        HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter();
+        exporter.setService(new AccountServiceImpl());
+        exporter.setServiceInterface(AccountService.class);
+        return exporter;
+    }
+}
+
+@SpringBootConfiguration
+class SpringBootTestConfiguration {
+
+    @Bean(name = "/unsafeHttpInvokerServiceExporter")
+    HttpInvokerServiceExporter unsafeHttpInvokerServiceExporter() {
+        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/UnsafeSpringExporterInConfigurationClass.expected

@@ -0,0 +1,4 @@
+| SpringExporterUnsafeDeserialization.java:12:32:12:63 | unsafeHttpInvokerServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeHttpInvokerServiceExporter' |
+| SpringExporterUnsafeDeserialization.java:20:41:20:88 | unsafeCustomeRemoteInvocationSerializingExporter | Unsafe deserialization in a Spring exporter bean '/unsafeCustomeRemoteInvocationSerializingExporter' |
+| SpringExporterUnsafeDeserialization.java:36:32:36:63 | unsafeHttpInvokerServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeHttpInvokerServiceExporter' |
+| SpringExporterUnsafeDeserialization.java:48:32:48:63 | unsafeHttpInvokerServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeHttpInvokerServiceExporter' |

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

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

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

@@ -0,0 +1,2 @@
+| beans.xml:10:5:13:12 | /unsafeBooking | Unsafe deserialization in a Spring exporter bean '/unsafeBooking' |
+| beans.xml:15:5:18:12 | org.springframework.remoting.httpinvoker.HttpInvokerServiceExporter | Unsafe deserialization in a Spring exporter bean 'org.springframework.remoting.httpinvoker.HttpInvokerServiceExporter' |

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

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

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

@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<beans xmlns="http://www.springframework.org/schema/beans"
+       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns:context="http://www.springframework.org/schema/context"
+       xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd
+	http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context.xsd">
+
+    <bean id="anotherBookingService" class="com.gypsyengineer.server.CabBookingServiceImpl"/>
+
+    <bean name="/unsafeBooking" class="org.springframework.remoting.httpinvoker.HttpInvokerServiceExporter">
+        <property name="service" ref="anotherBookingService"/>
+        <property name="serviceInterface" value="com.gypsyengineer.api.CabBookingService"/>
+    </bean>
+
+    <bean class="org.springframework.remoting.httpinvoker.HttpInvokerServiceExporter">
+        <property name="service" ref="anotherBookingService"/>
+        <property name="serviceInterface" value="com.gypsyengineer.api.CabBookingService"/>
+    </bean>
+</beans>

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/boot/SpringBootConfiguration.java

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

java/ql/test/stubs/springframework-5.2.3/org/springframework/boot/autoconfigure/SpringBootApplication.java

@@ -0,0 +1,12 @@
+package org.springframework.boot.autoconfigure;
+
+import java.lang.annotation.Target;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Inherited;
+
+import org.springframework.boot.SpringBootConfiguration;
+
+@Target(ElementType.TYPE)
+@Inherited
+@SpringBootConfiguration
+public @interface SpringBootApplication {}