Cover beans from XML configs in SpringHttpInvokerUnsafeDeserialization.ql

Author: artem-smotrakov

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

@@ -1,7 +1,8 @@
 /**
- * @name Unsafe deserialization with spring's remote service exporters.
- * @description Creating a bean based on RemoteInvocationSerializingExporter
- *              may lead to arbitrary code execution.
+ * @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
@@ -11,26 +12,14 @@
  */
 
 import java
-
-/**
- * Holds if `method` initializes a bean.
- */
-private predicate createsBean(Method method) {
-  method.hasAnnotation("org.springframework.context.annotation", "Bean")
-}
+import semmle.code.java.frameworks.spring.SpringBean
 
 /**
  * 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*())
+  type.getASupertype*()
+      .hasQualifiedName("org.springframework.remoting.rmi", "RemoteInvocationSerializingExporter")
 }
 
 /**
@@ -41,15 +30,37 @@ private predicate isInConfiguration(Method method) {
 }
 
 /**
- * Holds if `method` initializes a bean that is based on `RemoteInvocationSerializingExporter`.
+ * A method that initializes a unsafe bean based on `RemoteInvocationSerializingExporter`.
  */
-private predicate createsRemoteInvocationSerializingExporterBean(Method method) {
-  isInConfiguration(method) and
-  createsBean(method) and
-  returnsRemoteInvocationSerializingExporter(method)
+private class UnsafeBeanInitMethod extends Method {
+  string identifier;
+
+  UnsafeBeanInitMethod() {
+    isInConfiguration(this) and
+    isRemoteInvocationSerializingExporter(this.getReturnType()) and
+    exists(Annotation a |
+      a.getType().hasQualifiedName("org.springframework.context.annotation", "Bean")
+    |
+      this.getAnAnnotation() = a and
+      if a.getValue("name") instanceof StringLiteral
+      then identifier = a.getValue("name").(StringLiteral).getRepresentedString()
+      else identifier = this.getName()
+    )
+  }
+
+  string getBeanIdentifier() { result = identifier }
 }
 
-from Method method
-where createsRemoteInvocationSerializingExporterBean(method)
-select method,
-  "Unsafe deserialization in a remote service exporter in '" + method.getName() + "' method"
+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 + "'"

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

@@ -1,2 +1,4 @@
-| 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 |
+| SpringHttpInvokerUnsafeDeserialization.java:0:0:0:0 | SpringHttpInvokerUnsafeDeserialization | Unsafe deserialization in a remote service exporter bean '/unsafeCustomeRemoteInvocationSerializingExporter' |
+| SpringHttpInvokerUnsafeDeserialization.java:0:0:0:0 | SpringHttpInvokerUnsafeDeserialization | Unsafe deserialization in a remote service exporter bean '/unsafeHttpInvokerServiceExporter' |
+| beans.xml:0:0:0:0 | beans.xml | Unsafe deserialization in a remote service exporter bean '/unsafeBooking' |
+| beans.xml:0:0:0:0 | beans.xml | Unsafe deserialization in a remote service exporter bean 'org.springframework.remoting.httpinvoker.HttpInvokerServiceExporter' |

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

@@ -5,7 +5,7 @@
 
 @Configuration
 public class SpringHttpInvokerUnsafeDeserialization {
-    
+
     @Bean(name = "/unsafeHttpInvokerServiceExporter")
     HttpInvokerServiceExporter unsafeHttpInvokerServiceExporter() {
         HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter();

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>