Split SpringExporterUnsafeDeserialization.ql

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

@@ -46,10 +46,6 @@ The following example shows how a vulnerable HTTP endpoint can be defined
 using <code>HttpInvokerServiceExporter</code> and Spring annotations:
 </p>
 <sample src="SpringExporterUnsafeDeserialization.java" />
-<p>
-The next examples shows how the same vulnerable endpoint can be defined in a Spring XML config:
-</p>
-<sample src="SpringExporterUnsafeDeserialization.xml" />
 </example>
 
 <references>

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

@@ -6,28 +6,13 @@
  * @kind problem
  * @problem.severity error
  * @precision high
- * @id java/spring-exporter-unsafe-deserialization
+ * @id java/unsafe-deserialization-spring-exporter-in-configuration-class
  * @tags security
  *       external/cwe/cwe-502
  */
 
 import java
-import semmle.code.java.frameworks.spring.SpringBean
-
-/**
- * Holds if `type` is `RemoteInvocationSerializingExporter`.
- */
-private predicate isRemoteInvocationSerializingExporter(RefType type) {
-  type.getASupertype*()
-      .hasQualifiedName("org.springframework.remoting.rmi", "RemoteInvocationSerializingExporter")
-}
-
-/**
- * Holds if `method` belongs to a Spring configuration.
- */
-private predicate isInConfiguration(Method method) {
-  method.getDeclaringType().hasAnnotation("org.springframework.context.annotation", "Configuration")
-}
+import UnsafeSpringExporterLib
 
 /**
  * A method that initializes a unsafe bean based on `RemoteInvocationSerializingExporter`.
@@ -36,8 +21,8 @@ private class UnsafeBeanInitMethod extends Method {
   string identifier;
 
   UnsafeBeanInitMethod() {
-    isInConfiguration(this) and
     isRemoteInvocationSerializingExporter(this.getReturnType()) and
+    this.getDeclaringType().hasAnnotation("org.springframework.context.annotation", "Configuration") and
     exists(Annotation a |
       a.getType().hasQualifiedName("org.springframework.context.annotation", "Bean")
     |
@@ -51,16 +36,6 @@ private class UnsafeBeanInitMethod extends Method {
   string getBeanIdentifier() { result = identifier }
 }
 
-from File file, string identifier
-where
-  exists(UnsafeBeanInitMethod method |
-    file = method.getFile() and
-    identifier = method.getBeanIdentifier()
-  )
-  or
-  exists(SpringBean bean |
-    isRemoteInvocationSerializingExporter(bean.getClass()) and
-    file = bean.getFile() and
-    identifier = bean.getBeanIdentifier()
-  )
-select file, "Unsafe deserialization in Spring exporter bean '" + identifier + "'"
+from UnsafeBeanInitMethod method
+select method,
+  "Unsafe deserialization in a Spring exporter bean '" + method.getBeanIdentifier() + "'"

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

@@ -0,0 +1,81 @@
+<!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 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 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>
+
+<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>
+
+<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/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/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")
+}