From 61c521cb79dab40ba27e1292ac1315f54c053bd7 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Tue, 9 Mar 2021 00:02:57 +0300 Subject: [PATCH] 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 04ddac33aaf2f..87a57b0a43cb6 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 417f1c8790b74..ac68fcedcb5d7 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 * @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 0000000000000..6972e5eeb7de7 --- /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 next 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 0000000000000..49892a983e126 --- /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 + * @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 0000000000000..f49ee44304fd7 --- /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 1b5c1d2e67d9b..0000000000000 --- 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 7be52b73cba95..0000000000000 --- 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 0000000000000..926eff89403cd --- /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 0000000000000..823c7735ec5a7 --- /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 0000000000000..edcb3668557bb --- /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 0000000000000..46024a0b6b33a --- /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