Merge pull request github#6142 from artem-smotrakov/better-spring-exporters

Added sinks for RmiBasedExporter and HessianExporter

Author: smowton

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

@@ -5,5 +5,7 @@ import java
  */
 predicate isRemoteInvocationSerializingExporter(RefType type) {
   type.getASupertype*()
-      .hasQualifiedName("org.springframework.remoting.rmi", "RemoteInvocationSerializingExporter")
+      .hasQualifiedName("org.springframework.remoting.rmi",
+        ["RemoteInvocationSerializingExporter", "RmiBasedExporter"]) or
+  type.getASupertype*().hasQualifiedName("org.springframework.remoting.caucho", "HessianExporter")
 }

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

@@ -5,21 +5,18 @@
 
 <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>.
+The Spring Framework provides several classes for creating remote service exporters.
+Under the hood, the exporters use various deserialization mechanisms
+such as <code>ObjectInputStream</code> or Hessian.
 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>.
+to execute arbitrary code. If a remote attacker can reach endpoints created by the exporters,
+it results in remote code execution in the worst case.
 </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.
+Examples of unsafe exporters include: <code>HttpInvokerServiceExporter</code>, 
+<code>SimpleHttpInvokerServiceExporter</code>, <code>RmiServiceExporter</code>, 
+<code>HessianServiceExporter</code>.
 </p>
 <p>
 CVE-2016-1000027 has been assigned to this issue in the Spring Framework.
@@ -29,13 +26,11 @@ It is regarded as a design limitation, and can be mitigated but not fixed outrig
 
 <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),
+Avoid using unsafe service exporters. 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>
+</qhelp>

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

@@ -2,12 +2,32 @@
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.remoting.caucho.HessianServiceExporter;
 import org.springframework.remoting.httpinvoker.HttpInvokerServiceExporter;
 import org.springframework.remoting.rmi.RemoteInvocationSerializingExporter;
+import org.springframework.remoting.rmi.RmiServiceExporter;
 
 @Configuration
 public class SpringExporterUnsafeDeserialization {
 
+    @Bean(name = "/unsafeRmiServiceExporter")
+    RmiServiceExporter unsafeRmiServiceExporter() {
+        RmiServiceExporter exporter = new RmiServiceExporter();
+        exporter.setServiceInterface(AccountService.class);
+        exporter.setService(new AccountServiceImpl());
+        exporter.setServiceName(AccountService.class.getSimpleName());
+        exporter.setRegistryPort(1099); 
+        return exporter;
+    }
+
+    @Bean(name = "/unsafeHessianServiceExporter")
+    HessianServiceExporter unsafeHessianServiceExporter() {
+        HessianServiceExporter exporter = new HessianServiceExporter();
+        exporter.setService(new AccountServiceImpl());
+        exporter.setServiceInterface(AccountService.class);
+        return exporter;
+    }
+
     @Bean(name = "/unsafeHttpInvokerServiceExporter")
     HttpInvokerServiceExporter unsafeHttpInvokerServiceExporter() {
         HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter();

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

@@ -1,4 +1,6 @@
-| 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' |
+| SpringExporterUnsafeDeserialization.java:14:24:14:47 | unsafeRmiServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeRmiServiceExporter' |
+| SpringExporterUnsafeDeserialization.java:24:28:24:55 | unsafeHessianServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeHessianServiceExporter' |
+| SpringExporterUnsafeDeserialization.java:32:32:32:63 | unsafeHttpInvokerServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeHttpInvokerServiceExporter' |
+| SpringExporterUnsafeDeserialization.java:40:41:40:88 | unsafeCustomeRemoteInvocationSerializingExporter | Unsafe deserialization in a Spring exporter bean '/unsafeCustomeRemoteInvocationSerializingExporter' |
+| SpringExporterUnsafeDeserialization.java:56:32:56:63 | unsafeHttpInvokerServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeHttpInvokerServiceExporter' |
+| SpringExporterUnsafeDeserialization.java:68:32:68:63 | unsafeHttpInvokerServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeHttpInvokerServiceExporter' |

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

@@ -1,2 +1,4 @@
 | 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' |
+| beans.xml:20:5:24:12 | org.springframework.remoting.rmi.RmiServiceExporter | Unsafe deserialization in a Spring exporter bean 'org.springframework.remoting.rmi.RmiServiceExporter' |
+| beans.xml:26:5:29:12 | org.springframework.remoting.caucho.HessianServiceExporter | Unsafe deserialization in a Spring exporter bean 'org.springframework.remoting.caucho.HessianServiceExporter' |

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

@@ -16,4 +16,15 @@
         <property name="service" ref="anotherBookingService"/>
         <property name="serviceInterface" value="com.gypsyengineer.api.CabBookingService"/>
     </bean>
+
+    <bean class="org.springframework.remoting.rmi.RmiServiceExporter">
+        <property name="service" ref="oneMoreBookingService"/>
+        <property name="serviceInterface" value="com.gypsyengineer.api.CabBookingService"/>
+        <property name="registryPort" value="1199"/>
+    </bean>
+
+    <bean class="org.springframework.remoting.caucho.HessianServiceExporter">
+        <property name="service" ref="oneMoreBookingService"/>
+        <property name="serviceInterface" value="com.gypsyengineer.api.CabBookingService"/>
+    </bean>
 </beans>

java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/caucho/HessianExporter.java

@@ -0,0 +1,8 @@
+package org.springframework.remoting.caucho;
+
+public class HessianExporter {
+
+    public void setService(Object service) {}
+
+    public void setServiceInterface(Class clazz) {}
+}

java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/caucho/HessianServiceExporter.java

@@ -0,0 +1,3 @@
+package org.springframework.remoting.caucho;
+
+public class HessianServiceExporter extends HessianExporter {}

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

@@ -0,0 +1,12 @@
+package org.springframework.remoting.rmi;
+
+public abstract class RmiBasedExporter {
+
+    public void setService(Object service) {}
+
+    public void setServiceInterface(Class clazz) {}
+
+    public void setServiceName(String name) {}
+
+    public void setRegistryPort(int port) {}
+}

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

@@ -0,0 +1,3 @@
+package org.springframework.remoting.rmi;
+
+public class RmiServiceExporter extends RmiBasedExporter {}