Cover beans from XML configs in SpringHttpInvokerUnsafeDeserialization.ql

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 + "'"