From dcabce679abb6f9b9c1fcceb1ffc25e38484e05d Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Sat, 6 Mar 2021 21:40:35 +0100 Subject: [PATCH] Cover beans from XML configs in SpringHttpInvokerUnsafeDeserialization.ql --- .../SpringHttpInvokerUnsafeDeserialization.ql | 65 +++++++++++-------- ...gHttpInvokerUnsafeDeserialization.expected | 6 +- ...pringHttpInvokerUnsafeDeserialization.java | 2 +- .../query-tests/security/CWE-502/beans.xml | 19 ++++++ 4 files changed, 62 insertions(+), 30 deletions(-) create mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/beans.xml diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql b/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql index 54e8272575b..417f1c8790b 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql +++ b/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 + "'" diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.expected b/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.expected index b76f3edc57e..58fc508c1a4 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.expected +++ b/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' | diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.java b/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.java index 9d99aa1cbce..6811131fb5f 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.java +++ b/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.java @@ -5,7 +5,7 @@ import org.springframework.remoting.rmi.RemoteInvocationSerializingExporter; @Configuration public class SpringHttpInvokerUnsafeDeserialization { - + @Bean(name = "/unsafeHttpInvokerServiceExporter") HttpInvokerServiceExporter unsafeHttpInvokerServiceExporter() { HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter(); diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/beans.xml b/java/ql/test/experimental/query-tests/security/CWE-502/beans.xml new file mode 100644 index 00000000000..bdb4e5fa651 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-502/beans.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + +