Cover more configurations in UnsafeSpringExporterInConfigurationClass.ql

Author: artem-smotrakov

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

@@ -14,6 +14,22 @@
 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`.
  */
@@ -22,11 +38,9 @@ private class UnsafeBeanInitMethod extends Method {
 
   UnsafeBeanInitMethod() {
     isRemoteInvocationSerializingExporter(this.getReturnType()) and
-    this.getDeclaringType().hasAnnotation("org.springframework.context.annotation", "Configuration") and
-    exists(Annotation a |
-      a.getType().hasQualifiedName("org.springframework.context.annotation", "Bean")
-    |
-      this.getAnAnnotation() = a 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()

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

@@ -1,3 +1,5 @@
+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;
@@ -27,6 +29,30 @@ HttpInvokerServiceExporter notABean() {
     }
 }
 
+@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 {

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

@@ -1,2 +1,4 @@
-| SpringExporterUnsafeDeserialization.java:10:32:10:63 | unsafeHttpInvokerServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeHttpInvokerServiceExporter' |
-| SpringExporterUnsafeDeserialization.java:18:41:18:88 | unsafeCustomeRemoteInvocationSerializingExporter | Unsafe deserialization in a Spring exporter bean '/unsafeCustomeRemoteInvocationSerializingExporter' |
+| 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/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 {}