From 476309af6dc21295312598a6cd0fb406539dc2a6 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Sun, 21 Feb 2021 19:45:12 +0100 Subject: [PATCH 01/14] Added SpringHttpInvokerUnsafeDeserialization.ql --- .../SpringHttpInvokerUnsafeDeserialization.ql | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql b/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql new file mode 100644 index 000000000000..73c67582692b --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql @@ -0,0 +1,56 @@ +/** + * @name Unsafe deserialization with spring's remote service exporters. + * @description Creating a bean based on RemoteInvocationSerializingExporter + * may lead to arbitrary code execution. + * @kind problem + * @problem.severity error + * @precision high + * @id java/spring-exporter-unsafe-deserialization + * @tags security + * external/cwe/cwe-502 + */ + +import java + +/** + * Holds if `method` initializes a bean. + */ +private predicate createsBean(Method method) { + method.hasAnnotation("org.springframework.context.annotation", "Bean") +} + +/** + * 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()) or + isRemoteInvocationSerializingExporter(method.getReturnType().(RefType).getASupertype*()) +} + +/** + * Holds if `method` belongs to a Spring configuration. + */ +private predicate isInConfiguration(Method method) { + method.getDeclaringType().hasAnnotation("org.springframework.context.annotation", "Configuration") +} + +/** + * Holds if `method` initializes a bean that is based on `RemoteInvocationSerializingExporter`. + */ +private predicate createsRemoteInvocationSerializingExporterBean(Method method) { + isInConfiguration(method) and + createsBean(method) and + returnsRemoteInvocationSerializingExporter(method) +} + +from Method method +where createsRemoteInvocationSerializingExporterBean(method) +select method, + "Unasafe deserialization in a remote service exporter in '" + method.getName() + "' method" From 95284ad71ddc87e91baaf1dc4edd5d1494c8d806 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Sun, 21 Feb 2021 21:43:17 +0100 Subject: [PATCH 02/14] Added SpringHttpInvokerUnsafeDeserialization.qhelp and example --- ...ringHttpInvokerUnsafeDeserialization.qhelp | 69 +++++++++++++++++++ .../CWE-502/UnsafeHttpInvokerEndpoint.java | 24 +++++++ 2 files changed, 93 insertions(+) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp create mode 100644 java/ql/src/experimental/Security/CWE/CWE-502/UnsafeHttpInvokerEndpoint.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp new file mode 100644 index 000000000000..11a3333007ae --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp @@ -0,0 +1,69 @@ + + + + +

+Spring Framework provides an abstract base class RemoteInvocationSerializingExporter +for defining remote service exporters. +A Spring exporter, which is based on this class, deserializes incoming data using ObjectInputStream. +Deserializing untrusted data is easily exploitable and in many cases allows an attacker +to execute arbitrary code. +

+

+Spring Framework also provides two classes that extend RemoteInvocationSerializingExporter: +

  • +HttpInvokerServiceExporter +
    • +
  • +SimpleHttpInvokerServiceExporter +
    • +

    +

    +These classes export specified beans as HTTP endpoints that deserialize data from an HTTP request +using unsafe ObjectInputStream. If a remote attacker can reach such endpoints, +it results in remote code execution. +

    +     + + +

    +Avoid using HttpInvokerServiceExporter, SimpleHttpInvokerServiceExporter +and other exporters that are based on RemoteInvocationSerializingExporter. +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 by JEP 290. +In general, avoid deserialization of untrusted data. +

    +     + + +

    +The following example defines a vulnerable HTTP endpoint: +

    + +     + + +
  • +OWASP: +Deserialization of untrusted data. +
    • +
  • +National Vulnerability Database: +CVE-2016-1000027 +
    • +
  • +Tenable Research Advisory: +[R2] Pivotal Spring Framework HttpInvokerServiceExporter readRemoteInvocation Method Untrusted Java Deserialization +
    • +
  • +Spring Framework bug tracker: +Sonatype vulnerability CVE-2016-1000027 in Spring-web project +
    • +
  • +OpenJDK: +JEP 290: Filter Incoming Serialization Data +
    • +     +     \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeHttpInvokerEndpoint.java b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeHttpInvokerEndpoint.java new file mode 100644 index 000000000000..a4c6dadf17ca --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeHttpInvokerEndpoint.java @@ -0,0 +1,24 @@ +@Configuration +public class Server { + + @Bean(name = "/account") + HttpInvokerServiceExporter accountService() { + HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter(); + exporter.setService(new AccountServiceImpl()); + exporter.setServiceInterface(AccountService.class); + return exporter; + } + +} + +class AccountServiceImpl implements AccountService { + + @Override + public String echo(String data) { + return data; + } +} + +interface AccountService { + String echo(String data); +} \ No newline at end of file From aac0c27dcd25fc37b9a56b5235f2e7ea09404009 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Sun, 21 Feb 2021 22:19:53 +0100 Subject: [PATCH 03/14] Added tests for SpringHttpInvokerUnsafeDeserialization.ql --- ...gHttpInvokerUnsafeDeserialization.expected | 1 + ...pringHttpInvokerUnsafeDeserialization.java | 45 +++++++++++++++++++ ...ringHttpInvokerUnsafeDeserialization.qlref | 1 + .../query-tests/security/CWE-502/options | 1 + .../context/annotation/Bean.java | 10 +++++ .../context/annotation/Configuration.java | 7 +++ .../HttpInvokerServiceExporter.java | 8 ++++ .../RemoteInvocationSerializingExporter.java | 5 +++ 8 files changed, 78 insertions(+) create mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.expected create mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.java create mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.qlref create mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/options create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/context/annotation/Bean.java create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/context/annotation/Configuration.java create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/httpinvoker/HttpInvokerServiceExporter.java create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RemoteInvocationSerializingExporter.java 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 new file mode 100644 index 000000000000..0ce0efca8381 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.expected @@ -0,0 +1 @@ +| SpringHttpInvokerUnsafeDeserialization.java:9:32:9:37 | unsafe | Unasafe deserialization in a remote service exporter in 'unsafe' method | 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 new file mode 100644 index 000000000000..d678c8f01917 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.java @@ -0,0 +1,45 @@ +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.remoting.httpinvoker.HttpInvokerServiceExporter; + +@Configuration +public class SpringHttpInvokerUnsafeDeserialization { + + @Bean(name = "/unsafe") + HttpInvokerServiceExporter unsafe() { + HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter(); + exporter.setService(new AccountServiceImpl()); + exporter.setServiceInterface(AccountService.class); + return exporter; + } + + HttpInvokerServiceExporter notABean() { + HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter(); + exporter.setService(new AccountServiceImpl()); + exporter.setServiceInterface(AccountService.class); + return exporter; + } +} + +class NotAConfiguration { + + @Bean(name = "/notAnEndpoint") + HttpInvokerServiceExporter notAnEndpoint() { + HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter(); + exporter.setService(new AccountServiceImpl()); + exporter.setServiceInterface(AccountService.class); + return exporter; + } +} + +class AccountServiceImpl implements AccountService { + + @Override + public String echo(String data) { + return data; + } +} + +interface AccountService { + String echo(String data); +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.qlref b/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.qlref new file mode 100644 index 000000000000..014b0872ea47 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/options b/java/ql/test/experimental/query-tests/security/CWE-502/options new file mode 100644 index 000000000000..31b8e3f69351 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-502/options @@ -0,0 +1 @@ +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3 \ No newline at end of file diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/context/annotation/Bean.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/context/annotation/Bean.java new file mode 100644 index 000000000000..24d7be0f8174 --- /dev/null +++ b/java/ql/test/stubs/springframework-5.2.3/org/springframework/context/annotation/Bean.java @@ -0,0 +1,10 @@ +package org.springframework.context.annotation; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Target; + +@Target({ElementType.METHOD, ElementType.ANNOTATION_TYPE}) +public @interface Bean { + + String[] name() default {}; +} \ No newline at end of file diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/context/annotation/Configuration.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/context/annotation/Configuration.java new file mode 100644 index 000000000000..0024b3ba8338 --- /dev/null +++ b/java/ql/test/stubs/springframework-5.2.3/org/springframework/context/annotation/Configuration.java @@ -0,0 +1,7 @@ +package org.springframework.context.annotation; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Target; + +@Target(ElementType.TYPE) +public @interface Configuration {} \ No newline at end of file diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/httpinvoker/HttpInvokerServiceExporter.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/httpinvoker/HttpInvokerServiceExporter.java new file mode 100644 index 000000000000..5eb56c0897b2 --- /dev/null +++ b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/httpinvoker/HttpInvokerServiceExporter.java @@ -0,0 +1,8 @@ +package org.springframework.remoting.httpinvoker; + +public class HttpInvokerServiceExporter extends org.springframework.remoting.rmi.RemoteInvocationSerializingExporter { + + public void setService(Object service) {} + + public void setServiceInterface(Class clazz) {} +} \ No newline at end of file diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RemoteInvocationSerializingExporter.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RemoteInvocationSerializingExporter.java new file mode 100644 index 000000000000..a6f6fec194f8 --- /dev/null +++ b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RemoteInvocationSerializingExporter.java @@ -0,0 +1,5 @@ +package org.springframework.remoting.rmi; + +public abstract class RemoteInvocationSerializingExporter { + +} From e02b51f42bad96acbdf29a22c3768af894a942af Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 24 Feb 2021 22:23:11 +0100 Subject: [PATCH 04/14] Improved SpringHttpInvokerUnsafeDeserialization.qhelp --- .../SpringHttpInvokerUnsafeDeserialization.qhelp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp index 11a3333007ae..49237a8500ee 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp @@ -21,14 +21,17 @@ Spring Framework also provides two classes that extend RemoteInvocationSer

    These classes export specified beans as HTTP endpoints that deserialize data from an HTTP request using unsafe ObjectInputStream. If a remote attacker can reach such endpoints, -it results in remote code execution. +it results in remote code execution in the worst case. +

    +

    +CVE-2016-1000027 has been assigned to this issue in Spring Framework. There is no fix for that.

    Avoid using HttpInvokerServiceExporter, SimpleHttpInvokerServiceExporter -and other exporters that are based on RemoteInvocationSerializingExporter. +and any other exporter that is based on RemoteInvocationSerializingExporter. 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, @@ -50,6 +53,14 @@ OWASP: Deserialization of untrusted data.

  • +Spring Framework API documentation: +RemoteInvocationSerializingExporter class +
    • +
  • +Spring Framework API documentation: +HttpInvokerServiceExporter class +
    • +
  • National Vulnerability Database: CVE-2016-1000027
    • @@ -66,4 +77,5 @@ OpenJDK: JEP 290: Filter Incoming Serialization Data + \ No newline at end of file From 15a43ffe36e1a77eaf33ed17b3edbf6843dfc9df Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Sat, 27 Feb 2021 13:41:20 +0100 Subject: [PATCH 05/14] Simplified returnsRemoteInvocationSerializingExporter() --- .../SpringHttpInvokerUnsafeDeserialization.ql | 1 - .../SpringHttpInvokerUnsafeDeserialization.expected | 3 ++- .../SpringHttpInvokerUnsafeDeserialization.java | 12 ++++++++++-- .../rmi/RemoteInvocationSerializingExporter.java | 4 +--- 4 files changed, 13 insertions(+), 7 deletions(-) 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 73c67582692b..df01ae478cc7 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql @@ -30,7 +30,6 @@ private predicate isRemoteInvocationSerializingExporter(RefType type) { * Holds if `method` returns an object that extends `RemoteInvocationSerializingExporter`. */ private predicate returnsRemoteInvocationSerializingExporter(Method method) { - isRemoteInvocationSerializingExporter(method.getReturnType()) or isRemoteInvocationSerializingExporter(method.getReturnType().(RefType).getASupertype*()) } 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 0ce0efca8381..b76f3edc57e6 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 +1,2 @@ -| SpringHttpInvokerUnsafeDeserialization.java:9:32:9:37 | unsafe | Unasafe deserialization in a remote service exporter in 'unsafe' method | +| 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 | 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 d678c8f01917..9d99aa1cbce9 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 @@ -1,18 +1,24 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.remoting.httpinvoker.HttpInvokerServiceExporter; +import org.springframework.remoting.rmi.RemoteInvocationSerializingExporter; @Configuration public class SpringHttpInvokerUnsafeDeserialization { - @Bean(name = "/unsafe") - HttpInvokerServiceExporter unsafe() { + @Bean(name = "/unsafeHttpInvokerServiceExporter") + HttpInvokerServiceExporter unsafeHttpInvokerServiceExporter() { HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter(); exporter.setService(new AccountServiceImpl()); exporter.setServiceInterface(AccountService.class); return exporter; } + @Bean(name = "/unsafeCustomeRemoteInvocationSerializingExporter") + RemoteInvocationSerializingExporter unsafeCustomeRemoteInvocationSerializingExporter() { + return new CustomeRemoteInvocationSerializingExporter(); + } + HttpInvokerServiceExporter notABean() { HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter(); exporter.setService(new AccountServiceImpl()); @@ -21,6 +27,8 @@ HttpInvokerServiceExporter notABean() { } } +class CustomeRemoteInvocationSerializingExporter extends RemoteInvocationSerializingExporter {} + class NotAConfiguration { @Bean(name = "/notAnEndpoint") diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RemoteInvocationSerializingExporter.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RemoteInvocationSerializingExporter.java index a6f6fec194f8..5449b83ba865 100644 --- a/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RemoteInvocationSerializingExporter.java +++ b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RemoteInvocationSerializingExporter.java @@ -1,5 +1,3 @@ package org.springframework.remoting.rmi; -public abstract class RemoteInvocationSerializingExporter { - -} +public abstract class RemoteInvocationSerializingExporter {} From 617ba65ef5994e285a26bc4ac0a799d37b3fdb60 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Tue, 2 Mar 2021 21:36:14 +0100 Subject: [PATCH 06/14] Improved docs for SpringHttpInvokerUnsafeDeserialization.ql --- .../CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp | 8 ++++---- .../CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp index 49237a8500ee..ffb8dddae566 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp @@ -3,14 +3,14 @@

    -Spring Framework provides an abstract base class RemoteInvocationSerializingExporter +The Spring Framework provides an abstract base class RemoteInvocationSerializingExporter for defining remote service exporters. A Spring exporter, which is based on this class, deserializes incoming data using ObjectInputStream. Deserializing untrusted data is easily exploitable and in many cases allows an attacker to execute arbitrary code.

    -Spring Framework also provides two classes that extend RemoteInvocationSerializingExporter: +The Spring Framework also provides two classes that extend RemoteInvocationSerializingExporter:

  • HttpInvokerServiceExporter
    • @@ -24,7 +24,7 @@ using unsafe ObjectInputStream. If a remote attacker can reach such it results in remote code execution in the worst case.

    -CVE-2016-1000027 has been assigned to this issue in Spring Framework. There is no fix for that. +CVE-2016-1000027 has been assigned to this issue in the Spring Framework. It is regarded as a design limitation, and can be mitigated but not fixed outright.

    @@ -36,7 +36,7 @@ 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 by JEP 290. -In general, avoid deserialization of untrusted data. +In general, avoid using Java's built-in deserialization methods on untrusted data.

    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 df01ae478cc7..54e8272575b6 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql @@ -52,4 +52,4 @@ private predicate createsRemoteInvocationSerializingExporterBean(Method method) from Method method where createsRemoteInvocationSerializingExporterBean(method) select method, - "Unasafe deserialization in a remote service exporter in '" + method.getName() + "' method" + "Unsafe deserialization in a remote service exporter in '" + method.getName() + "' method" From dcabce679abb6f9b9c1fcceb1ffc25e38484e05d Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Sat, 6 Mar 2021 21:40:35 +0100 Subject: [PATCH 07/14] 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 54e8272575b6..417f1c8790b7 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 b76f3edc57e6..58fc508c1a45 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 9d99aa1cbce9..6811131fb5f9 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 @@ @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 000000000000..bdb4e5fa6515 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-502/beans.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + From 82cb4a8d68a76d6abd909e88d243874afb515407 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Sat, 6 Mar 2021 21:48:35 +0100 Subject: [PATCH 08/14] Renamed SpringHttpInvokerUnsafeDeserialization.ql --- ...Endpoint.java => SpringExporterUnsafeDeserialization.java} | 0 ...zation.qhelp => SpringExporterUnsafeDeserialization.qhelp} | 0 ...erialization.ql => SpringExporterUnsafeDeserialization.ql} | 0 .../CWE-502/SpringExporterUnsafeDeserialization.expected | 4 ++++ ...lization.java => SpringExporterUnsafeDeserialization.java} | 2 +- .../CWE-502/SpringExporterUnsafeDeserialization.qlref | 1 + .../CWE-502/SpringHttpInvokerUnsafeDeserialization.expected | 4 ---- .../CWE-502/SpringHttpInvokerUnsafeDeserialization.qlref | 1 - 8 files changed, 6 insertions(+), 6 deletions(-) rename java/ql/src/experimental/Security/CWE/CWE-502/{UnsafeHttpInvokerEndpoint.java => SpringExporterUnsafeDeserialization.java} (100%) rename java/ql/src/experimental/Security/CWE/CWE-502/{SpringHttpInvokerUnsafeDeserialization.qhelp => SpringExporterUnsafeDeserialization.qhelp} (100%) rename java/ql/src/experimental/Security/CWE/CWE-502/{SpringHttpInvokerUnsafeDeserialization.ql => SpringExporterUnsafeDeserialization.ql} (100%) create mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.expected rename java/ql/test/experimental/query-tests/security/CWE-502/{SpringHttpInvokerUnsafeDeserialization.java => SpringExporterUnsafeDeserialization.java} (97%) create mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.qlref delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.expected delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.qlref diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeHttpInvokerEndpoint.java b/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.java similarity index 100% rename from java/ql/src/experimental/Security/CWE/CWE-502/UnsafeHttpInvokerEndpoint.java rename to java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp similarity index 100% rename from java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.qhelp rename to java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql b/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.ql similarity index 100% rename from java/ql/src/experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql rename to java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.ql diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.expected b/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.expected new file mode 100644 index 000000000000..1b5c1d2e67d9 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.expected @@ -0,0 +1,4 @@ +| SpringExporterUnsafeDeserialization.java:0:0:0:0 | SpringExporterUnsafeDeserialization | Unsafe deserialization in Spring exporter bean '/unsafeCustomeRemoteInvocationSerializingExporter' | +| SpringExporterUnsafeDeserialization.java:0:0:0:0 | SpringExporterUnsafeDeserialization | Unsafe deserialization in Spring exporter bean '/unsafeHttpInvokerServiceExporter' | +| beans.xml:0:0:0:0 | beans.xml | Unsafe deserialization in Spring exporter bean '/unsafeBooking' | +| beans.xml:0:0:0:0 | beans.xml | Unsafe deserialization in Spring 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/SpringExporterUnsafeDeserialization.java similarity index 97% rename from java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.java rename to java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.java index 6811131fb5f9..c89668fc6bbc 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.java +++ b/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.java @@ -4,7 +4,7 @@ import org.springframework.remoting.rmi.RemoteInvocationSerializingExporter; @Configuration -public class SpringHttpInvokerUnsafeDeserialization { +public class SpringExporterUnsafeDeserialization { @Bean(name = "/unsafeHttpInvokerServiceExporter") HttpInvokerServiceExporter unsafeHttpInvokerServiceExporter() { diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.qlref b/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.qlref new file mode 100644 index 000000000000..7be52b73cba9 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.ql \ No newline at end of file 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 deleted file mode 100644 index 58fc508c1a45..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.expected +++ /dev/null @@ -1,4 +0,0 @@ -| 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.qlref b/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.qlref deleted file mode 100644 index 014b0872ea47..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-502/SpringHttpInvokerUnsafeDeserialization.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/Security/CWE/CWE-502/SpringHttpInvokerUnsafeDeserialization.ql \ No newline at end of file From bda223771bddcdc2d67b86714d76d0faf895ffda Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Sat, 6 Mar 2021 22:05:00 +0100 Subject: [PATCH 09/14] Added another example for SpringExporterUnsafeDeserialization.ql --- .../SpringExporterUnsafeDeserialization.qhelp | 15 ++++++++++----- .../SpringExporterUnsafeDeserialization.xml | 4 ++++ 2 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.xml diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp index ffb8dddae566..da681b7125ca 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp @@ -4,7 +4,7 @@

    The Spring Framework provides an abstract base class RemoteInvocationSerializingExporter -for defining remote service exporters. +for creating remote service exporters. A Spring exporter, which is based on this class, deserializes incoming data using ObjectInputStream. Deserializing untrusted data is easily exploitable and in many cases allows an attacker to execute arbitrary code. @@ -24,7 +24,8 @@ using unsafe ObjectInputStream. If a remote attacker can reach such it results in remote code execution in the worst case.

    -CVE-2016-1000027 has been assigned to this issue in the Spring Framework. It is regarded as a design limitation, and can be mitigated but not fixed outright. +CVE-2016-1000027 has been assigned to this issue in the Spring Framework. +It is regarded as a design limitation, and can be mitigated but not fixed outright.

    @@ -35,16 +36,20 @@ and any other exporter that is based on RemoteInvocationSerializingExporte 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 by JEP 290. -In general, avoid using Java's built-in deserialization methods on untrusted data. +consider using global deserialization filters introduced in JEP 290.

    -The following example defines a vulnerable HTTP endpoint: +The following example shows how a vulnerable HTTP endpoint can be defined +using HttpInvokerServiceExporter and Spring annotations:

    +

    +The next examples shows how the same vulnerable endpoint can be defined in a Spring XML config: +

    +     diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.xml b/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.xml new file mode 100644 index 000000000000..bf058cfffc1c --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.xml @@ -0,0 +1,4 @@ + + + + \ No newline at end of file From 891b975899bafae5e8777c2f0b9de4c299c7835b Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Sat, 6 Mar 2021 22:07:43 +0100 Subject: [PATCH 10/14] Use correct file names in SpringExporterUnsafeDeserialization.qhelp --- .../CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp index da681b7125ca..04ddac33aaf2 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp @@ -45,11 +45,11 @@ consider using global deserialization filters introduced in JEP 290. The following example shows how a vulnerable HTTP endpoint can be defined using HttpInvokerServiceExporter and Spring annotations:

    - +

    The next examples shows how the same vulnerable endpoint can be defined in a Spring XML config:

    - + From a78f2115f22cf0becb7214bc4fed48c75bbfa617 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Tue, 9 Mar 2021 00:02:57 +0300 Subject: [PATCH 11/14] Split SpringExporterUnsafeDeserialization.ql --- ...eSpringExporterInConfigurationClass.qhelp} | 4 - ...safeSpringExporterInConfigurationClass.ql} | 37 ++------- ...safeSpringExporterInXMLConfiguration.qhelp | 81 +++++++++++++++++++ .../UnsafeSpringExporterInXMLConfiguration.ql | 20 +++++ .../CWE/CWE-502/UnsafeSpringExporterLib.qll | 9 +++ ...ringExporterUnsafeDeserialization.expected | 4 - .../SpringExporterUnsafeDeserialization.qlref | 1 - ...pringExporterInConfigurationClass.expected | 2 + ...feSpringExporterInConfigurationClass.qlref | 1 + ...eSpringExporterInXMLConfiguration.expected | 2 + ...safeSpringExporterInXMLConfiguration.qlref | 1 + 11 files changed, 122 insertions(+), 40 deletions(-) rename java/ql/src/experimental/Security/CWE/CWE-502/{SpringExporterUnsafeDeserialization.qhelp => UnsafeSpringExporterInConfigurationClass.qhelp} (95%) rename java/ql/src/experimental/Security/CWE/CWE-502/{SpringExporterUnsafeDeserialization.ql => UnsafeSpringExporterInConfigurationClass.ql} (52%) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfiguration.qhelp create mode 100644 java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfiguration.ql create mode 100644 java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterLib.qll delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.expected delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.qlref create mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInConfigurationClass.expected create mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInConfigurationClass.qlref create mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInXMLConfiguration.expected create mode 100644 java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInXMLConfiguration.qlref diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.qhelp similarity index 95% rename from java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp rename to java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.qhelp index 04ddac33aaf2..87a57b0a43cb 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.qhelp @@ -46,10 +46,6 @@ The following example shows how a vulnerable HTTP endpoint can be defined using HttpInvokerServiceExporter and Spring annotations:

    -

    -The next examples shows how the same vulnerable endpoint can be defined in a Spring XML config: -

    - diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.ql b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.ql similarity index 52% rename from java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.ql rename to java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.ql index 417f1c8790b7..34605d16fbdd 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.ql @@ -6,28 +6,13 @@ * @kind problem * @problem.severity error * @precision high - * @id java/spring-exporter-unsafe-deserialization + * @id java/unsafe-deserialization-spring-exporter-in-configuration-class * @tags security * external/cwe/cwe-502 */ import java -import semmle.code.java.frameworks.spring.SpringBean - -/** - * Holds if `type` is `RemoteInvocationSerializingExporter`. - */ -private predicate isRemoteInvocationSerializingExporter(RefType type) { - type.getASupertype*() - .hasQualifiedName("org.springframework.remoting.rmi", "RemoteInvocationSerializingExporter") -} - -/** - * Holds if `method` belongs to a Spring configuration. - */ -private predicate isInConfiguration(Method method) { - method.getDeclaringType().hasAnnotation("org.springframework.context.annotation", "Configuration") -} +import UnsafeSpringExporterLib /** * A method that initializes a unsafe bean based on `RemoteInvocationSerializingExporter`. @@ -36,8 +21,8 @@ private class UnsafeBeanInitMethod extends Method { string identifier; UnsafeBeanInitMethod() { - isInConfiguration(this) and isRemoteInvocationSerializingExporter(this.getReturnType()) and + this.getDeclaringType().hasAnnotation("org.springframework.context.annotation", "Configuration") and exists(Annotation a | a.getType().hasQualifiedName("org.springframework.context.annotation", "Bean") | @@ -51,16 +36,6 @@ private class UnsafeBeanInitMethod extends Method { string getBeanIdentifier() { result = identifier } } -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 + "'" +from UnsafeBeanInitMethod method +select method, + "Unsafe deserialization in a Spring exporter bean '" + method.getBeanIdentifier() + "'" diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfiguration.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfiguration.qhelp new file mode 100644 index 000000000000..dcefdfb97b06 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfiguration.qhelp @@ -0,0 +1,81 @@ + + + + +

    +The Spring Framework provides an abstract base class RemoteInvocationSerializingExporter +for creating remote service exporters. +A Spring exporter, which is based on this class, deserializes incoming data using ObjectInputStream. +Deserializing untrusted data is easily exploitable and in many cases allows an attacker +to execute arbitrary code. +

    +

    +The Spring Framework also provides two classes that extend RemoteInvocationSerializingExporter: +

  • +HttpInvokerServiceExporter +
    • +
  • +SimpleHttpInvokerServiceExporter +
    • +

    +

    +These classes export specified beans as HTTP endpoints that deserialize data from an HTTP request +using unsafe ObjectInputStream. If a remote attacker can reach such endpoints, +it results in remote code execution in the worst case. +

    +

    +CVE-2016-1000027 has been assigned to this issue in the Spring Framework. +It is regarded as a design limitation, and can be mitigated but not fixed outright. +

    +     + + +

    +Avoid using HttpInvokerServiceExporter, SimpleHttpInvokerServiceExporter +and any other exporter that is based on RemoteInvocationSerializingExporter. +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. +

    +     + + +

    +The following examples shows how a vulnerable HTTP endpoint can be defined in a Spring XML config: +

    + +     + + +
  • +OWASP: +Deserialization of untrusted data. +
    • +
  • +Spring Framework API documentation: +RemoteInvocationSerializingExporter class +
    • +
  • +Spring Framework API documentation: +HttpInvokerServiceExporter class +
    • +
  • +National Vulnerability Database: +CVE-2016-1000027 +
    • +
  • +Tenable Research Advisory: +[R2] Pivotal Spring Framework HttpInvokerServiceExporter readRemoteInvocation Method Untrusted Java Deserialization +
    • +
  • +Spring Framework bug tracker: +Sonatype vulnerability CVE-2016-1000027 in Spring-web project +
    • +
  • +OpenJDK: +JEP 290: Filter Incoming Serialization Data +
    • +     + +     \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfiguration.ql b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfiguration.ql new file mode 100644 index 000000000000..d7606587df35 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfiguration.ql @@ -0,0 +1,20 @@ +/** + * @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 + * @id java/unsafe-deserialization-spring-exporter-in-xml-configuration + * @tags security + * external/cwe/cwe-502 + */ + +import java +import semmle.code.java.frameworks.spring.SpringBean +import UnsafeSpringExporterLib + +from SpringBean bean +where isRemoteInvocationSerializingExporter(bean.getClass()) +select bean, "Unsafe deserialization in a Spring exporter bean '" + bean.getBeanIdentifier() + "'" diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterLib.qll b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterLib.qll new file mode 100644 index 000000000000..f49ee44304fd --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterLib.qll @@ -0,0 +1,9 @@ +import java + +/** + * Holds if `type` is `RemoteInvocationSerializingExporter`. + */ +predicate isRemoteInvocationSerializingExporter(RefType type) { + type.getASupertype*() + .hasQualifiedName("org.springframework.remoting.rmi", "RemoteInvocationSerializingExporter") +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.expected b/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.expected deleted file mode 100644 index 1b5c1d2e67d9..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.expected +++ /dev/null @@ -1,4 +0,0 @@ -| SpringExporterUnsafeDeserialization.java:0:0:0:0 | SpringExporterUnsafeDeserialization | Unsafe deserialization in Spring exporter bean '/unsafeCustomeRemoteInvocationSerializingExporter' | -| SpringExporterUnsafeDeserialization.java:0:0:0:0 | SpringExporterUnsafeDeserialization | Unsafe deserialization in Spring exporter bean '/unsafeHttpInvokerServiceExporter' | -| beans.xml:0:0:0:0 | beans.xml | Unsafe deserialization in Spring exporter bean '/unsafeBooking' | -| beans.xml:0:0:0:0 | beans.xml | Unsafe deserialization in Spring exporter bean 'org.springframework.remoting.httpinvoker.HttpInvokerServiceExporter' | diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.qlref b/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.qlref deleted file mode 100644 index 7be52b73cba9..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/Security/CWE/CWE-502/SpringExporterUnsafeDeserialization.ql \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInConfigurationClass.expected b/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInConfigurationClass.expected new file mode 100644 index 000000000000..926eff89403c --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInConfigurationClass.expected @@ -0,0 +1,2 @@ +| 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' | diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInConfigurationClass.qlref b/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInConfigurationClass.qlref new file mode 100644 index 000000000000..823c7735ec5a --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInConfigurationClass.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.ql \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInXMLConfiguration.expected b/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInXMLConfiguration.expected new file mode 100644 index 000000000000..edcb3668557b --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInXMLConfiguration.expected @@ -0,0 +1,2 @@ +| 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' | diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInXMLConfiguration.qlref b/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInXMLConfiguration.qlref new file mode 100644 index 000000000000..46024a0b6b33 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInXMLConfiguration.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfiguration.ql \ No newline at end of file From df60268023493fe0fa6a4aa723f22435fb769451 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 10 Mar 2021 10:38:08 +0300 Subject: [PATCH 12/14] Split qhelp files --- ...feSpringExporterInConfigurationClass.qhelp | 86 ++----------------- ...orterInConfigurationClassExample.inc.qhelp | 14 +++ ...safeSpringExporterInXMLConfiguration.qhelp | 85 ++---------------- ...xporterInXMLConfigurationExample.inc.qhelp | 13 +++ .../UnsafeSpringExporterQuery.inc.qhelp | 41 +++++++++ .../UnsafeSpringExporterReferences.inc.qhelp | 37 ++++++++ 6 files changed, 117 insertions(+), 159 deletions(-) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClassExample.inc.qhelp create mode 100644 java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfigurationExample.inc.qhelp create mode 100644 java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterQuery.inc.qhelp create mode 100644 java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterReferences.inc.qhelp diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.qhelp index 87a57b0a43cb..8580092b8a1a 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.qhelp @@ -1,82 +1,8 @@ - + - - -

    -The Spring Framework provides an abstract base class RemoteInvocationSerializingExporter -for creating remote service exporters. -A Spring exporter, which is based on this class, deserializes incoming data using ObjectInputStream. -Deserializing untrusted data is easily exploitable and in many cases allows an attacker -to execute arbitrary code. -

    -

    -The Spring Framework also provides two classes that extend RemoteInvocationSerializingExporter: -

  • -HttpInvokerServiceExporter -
    • -
  • -SimpleHttpInvokerServiceExporter -
    • -

    -

    -These classes export specified beans as HTTP endpoints that deserialize data from an HTTP request -using unsafe ObjectInputStream. If a remote attacker can reach such endpoints, -it results in remote code execution in the worst case. -

    -

    -CVE-2016-1000027 has been assigned to this issue in the Spring Framework. -It is regarded as a design limitation, and can be mitigated but not fixed outright. -

    -     - - -

    -Avoid using HttpInvokerServiceExporter, SimpleHttpInvokerServiceExporter -and any other exporter that is based on RemoteInvocationSerializingExporter. -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. -

    -     - - -

    -The following example shows how a vulnerable HTTP endpoint can be defined -using HttpInvokerServiceExporter and Spring annotations: -

    - -     - - -
  • -OWASP: -Deserialization of untrusted data. -
    • -
  • -Spring Framework API documentation: -RemoteInvocationSerializingExporter class -
    • -
  • -Spring Framework API documentation: -HttpInvokerServiceExporter class -
    • -
  • -National Vulnerability Database: -CVE-2016-1000027 -
    • -
  • -Tenable Research Advisory: -[R2] Pivotal Spring Framework HttpInvokerServiceExporter readRemoteInvocation Method Untrusted Java Deserialization -
    • -
  • -Spring Framework bug tracker: -Sonatype vulnerability CVE-2016-1000027 in Spring-web project -
    • -
  • -OpenJDK: -JEP 290: Filter Incoming Serialization Data -
    • -     - + + +     \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClassExample.inc.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClassExample.inc.qhelp new file mode 100644 index 000000000000..ed33a03fabed --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClassExample.inc.qhelp @@ -0,0 +1,14 @@ + + + + +

    +The following example shows how a vulnerable HTTP endpoint can be defined +using HttpInvokerServiceExporter and Spring annotations: +

    + +     + +     \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfiguration.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfiguration.qhelp index dcefdfb97b06..76dda5842b3a 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfiguration.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfiguration.qhelp @@ -1,81 +1,8 @@ - + - - -

    -The Spring Framework provides an abstract base class RemoteInvocationSerializingExporter -for creating remote service exporters. -A Spring exporter, which is based on this class, deserializes incoming data using ObjectInputStream. -Deserializing untrusted data is easily exploitable and in many cases allows an attacker -to execute arbitrary code. -

    -

    -The Spring Framework also provides two classes that extend RemoteInvocationSerializingExporter: -

  • -HttpInvokerServiceExporter -
    • -
  • -SimpleHttpInvokerServiceExporter -
    • -

    -

    -These classes export specified beans as HTTP endpoints that deserialize data from an HTTP request -using unsafe ObjectInputStream. If a remote attacker can reach such endpoints, -it results in remote code execution in the worst case. -

    -

    -CVE-2016-1000027 has been assigned to this issue in the Spring Framework. -It is regarded as a design limitation, and can be mitigated but not fixed outright. -

    -     - - -

    -Avoid using HttpInvokerServiceExporter, SimpleHttpInvokerServiceExporter -and any other exporter that is based on RemoteInvocationSerializingExporter. -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. -

    -     - - -

    -The following examples shows how a vulnerable HTTP endpoint can be defined in a Spring XML config: -

    - -     - - -
  • -OWASP: -Deserialization of untrusted data. -
    • -
  • -Spring Framework API documentation: -RemoteInvocationSerializingExporter class -
    • -
  • -Spring Framework API documentation: -HttpInvokerServiceExporter class -
    • -
  • -National Vulnerability Database: -CVE-2016-1000027 -
    • -
  • -Tenable Research Advisory: -[R2] Pivotal Spring Framework HttpInvokerServiceExporter readRemoteInvocation Method Untrusted Java Deserialization -
    • -
  • -Spring Framework bug tracker: -Sonatype vulnerability CVE-2016-1000027 in Spring-web project -
    • -
  • -OpenJDK: -JEP 290: Filter Incoming Serialization Data -
    • -     - + + +     \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfigurationExample.inc.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfigurationExample.inc.qhelp new file mode 100644 index 000000000000..bc18f4dc2334 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInXMLConfigurationExample.inc.qhelp @@ -0,0 +1,13 @@ + + + + +

    +The following examples shows how a vulnerable HTTP endpoint can be defined in a Spring XML config: +

    + +     + +     \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterQuery.inc.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterQuery.inc.qhelp new file mode 100644 index 000000000000..732c5c7e545e --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterQuery.inc.qhelp @@ -0,0 +1,41 @@ + + + + +

    +The Spring Framework provides an abstract base class RemoteInvocationSerializingExporter +for creating remote service exporters. +A Spring exporter, which is based on this class, deserializes incoming data using ObjectInputStream. +Deserializing untrusted data is easily exploitable and in many cases allows an attacker +to execute arbitrary code. +

    +

    +The Spring Framework also provides HttpInvokerServiceExporter +and SimpleHttpInvokerServiceExporter classes +that extend RemoteInvocationSerializingExporter. +

    +

    +These classes export specified beans as HTTP endpoints that deserialize data from an HTTP request +using unsafe ObjectInputStream. If a remote attacker can reach such endpoints, +it results in remote code execution in the worst case. +

    +

    +CVE-2016-1000027 has been assigned to this issue in the Spring Framework. +It is regarded as a design limitation, and can be mitigated but not fixed outright. +

    +     + + +

    +Avoid using HttpInvokerServiceExporter, SimpleHttpInvokerServiceExporter +and any other exporter that is based on RemoteInvocationSerializingExporter. +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. +

    +     + +     \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterReferences.inc.qhelp b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterReferences.inc.qhelp new file mode 100644 index 000000000000..94d269e35d0d --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterReferences.inc.qhelp @@ -0,0 +1,37 @@ + + + + +
  • +OWASP: +Deserialization of untrusted data. +
    • +
  • +Spring Framework API documentation: +RemoteInvocationSerializingExporter class +
    • +
  • +Spring Framework API documentation: +HttpInvokerServiceExporter class +
    • +
  • +National Vulnerability Database: +CVE-2016-1000027 +
    • +
  • +Tenable Research Advisory: +[R2] Pivotal Spring Framework HttpInvokerServiceExporter readRemoteInvocation Method Untrusted Java Deserialization +
    • +
  • +Spring Framework bug tracker: +Sonatype vulnerability CVE-2016-1000027 in Spring-web project +
    • +
  • +OpenJDK: +JEP 290: Filter Incoming Serialization Data +
    • +     + +     \ No newline at end of file From 0a5d58ed8a15288a19394d74d39610a87869d864 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 10 Mar 2021 21:15:19 +0300 Subject: [PATCH 13/14] Cover more configurations in UnsafeSpringExporterInConfigurationClass.ql --- ...nsafeSpringExporterInConfigurationClass.ql | 24 +++++++++++++---- .../SpringExporterUnsafeDeserialization.java | 26 +++++++++++++++++++ ...pringExporterInConfigurationClass.expected | 6 +++-- .../boot/SpringBootConfiguration.java | 10 +++++++ .../autoconfigure/SpringBootApplication.java | 12 +++++++++ 5 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/boot/SpringBootConfiguration.java create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/boot/autoconfigure/SpringBootApplication.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.ql b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.ql index 34605d16fbdd..4b8fee3c83fd 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.ql +++ b/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() diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.java b/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.java index c89668fc6bbc..2bca24ee370e 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-502/SpringExporterUnsafeDeserialization.java +++ b/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 { diff --git a/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInConfigurationClass.expected b/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInConfigurationClass.expected index 926eff89403c..2155d805e80a 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInConfigurationClass.expected +++ b/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' | diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/boot/SpringBootConfiguration.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/boot/SpringBootConfiguration.java new file mode 100644 index 000000000000..483c5cc56065 --- /dev/null +++ b/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 {} \ No newline at end of file diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/boot/autoconfigure/SpringBootApplication.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/boot/autoconfigure/SpringBootApplication.java new file mode 100644 index 000000000000..38bbc35733e9 --- /dev/null +++ b/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 {} \ No newline at end of file From 4b7c57c077ff17eae1b94f69d90e67b1eb28e7ea Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Thu, 11 Mar 2021 11:52:07 +0100 Subject: [PATCH 14/14] Added a comment for getBeanIdentifier() Co-authored-by: Chris Smowton --- .../CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.ql | 3 +++ 1 file changed, 3 insertions(+) diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.ql b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.ql index 4b8fee3c83fd..81b56ce0e527 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterInConfigurationClass.ql @@ -47,6 +47,9 @@ private class UnsafeBeanInitMethod extends Method { ) } + /** + * Gets this bean's name if given by the `Bean` annotation, or this method's identifier otherwise. + */ string getBeanIdentifier() { result = identifier } }