From 14e724bce6cf3642699404c46bcc4ed98a63c457 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 23 Jun 2021 09:53:47 +0200 Subject: [PATCH 1/2] Added sinks for RmiBasedExporter and HessianExporter --- .../CWE/CWE-502/UnsafeSpringExporterLib.qll | 4 ++- .../UnsafeSpringExporterQuery.inc.qhelp | 26 ++++++++----------- .../SpringExporterUnsafeDeserialization.java | 20 ++++++++++++++ ...pringExporterInConfigurationClass.expected | 10 ++++--- ...eSpringExporterInXMLConfiguration.expected | 2 ++ .../query-tests/security/CWE-502/beans.xml | 11 ++++++++ .../remoting/caucho/HessianExporter.java | 8 ++++++ .../caucho/HessianServiceExporter.java | 3 +++ .../remoting/rmi/RmiBasedExporter.java | 12 +++++++++ .../remoting/rmi/RmiServiceExporter.java | 3 +++ 10 files changed, 79 insertions(+), 20 deletions(-) create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/caucho/HessianExporter.java create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/caucho/HessianServiceExporter.java create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RmiBasedExporter.java create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RmiServiceExporter.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterLib.qll b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterLib.qll index f49ee44304fd..414f9d21b518 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterLib.qll @@ -5,5 +5,7 @@ import java */ predicate isRemoteInvocationSerializingExporter(RefType type) { type.getASupertype*() - .hasQualifiedName("org.springframework.remoting.rmi", "RemoteInvocationSerializingExporter") + .hasQualifiedName("org.springframework.remoting.rmi", + ["RemoteInvocationSerializingExporter", "RmiBasedExporter"]) or + type.getASupertype*().hasQualifiedName("org.springframework.remoting.caucho", "HessianExporter") } 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 index 732c5c7e545e..f7acc417dc84 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterQuery.inc.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterQuery.inc.qhelp @@ -5,22 +5,20 @@

-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. +The Spring Framework provides several classes for creating remote service exporters. +Under the hood, the exporters use various deserialization mechanisms +such as ObjectInputStream or Hessian. 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. +to execute arbitrary code. If a remote attacker can reach endpoints created by the exporters, +it results in remote code execution in the worst case.

+

-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. +Here are examples of unsafe exporters: HttpInvokerServiceExporter, +SimpleHttpInvokerServiceExporter, RmiServiceExporter, +HessianServiceExporter.

+

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. @@ -29,9 +27,7 @@ It is regarded as a design limitation, and can be mitigated but not fixed outrig

-Avoid using HttpInvokerServiceExporter, SimpleHttpInvokerServiceExporter -and any other exporter that is based on RemoteInvocationSerializingExporter. -Instead, use other message formats for API endpoints (for example, JSON), +Avoid using unsafe service exporters. 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. 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 2bca24ee370e..f1b2453ea151 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 @@ -2,12 +2,32 @@ import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.remoting.caucho.HessianServiceExporter; import org.springframework.remoting.httpinvoker.HttpInvokerServiceExporter; import org.springframework.remoting.rmi.RemoteInvocationSerializingExporter; +import org.springframework.remoting.rmi.RmiServiceExporter; @Configuration public class SpringExporterUnsafeDeserialization { + @Bean(name = "/unsafeRmiServiceExporter") + RmiServiceExporter unsafeRmiServiceExporter() { + RmiServiceExporter exporter = new RmiServiceExporter(); + exporter.setServiceInterface(AccountService.class); + exporter.setService(new AccountServiceImpl()); + exporter.setServiceName(AccountService.class.getSimpleName()); + exporter.setRegistryPort(1099); + return exporter; + } + + @Bean(name = "/unsafeHessianServiceExporter") + HessianServiceExporter unsafeHessianServiceExporter() { + HessianServiceExporter exporter = new HessianServiceExporter(); + exporter.setService(new AccountServiceImpl()); + exporter.setServiceInterface(AccountService.class); + return exporter; + } + @Bean(name = "/unsafeHttpInvokerServiceExporter") HttpInvokerServiceExporter unsafeHttpInvokerServiceExporter() { HttpInvokerServiceExporter exporter = new HttpInvokerServiceExporter(); 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 2155d805e80a..0c266dfaf7e4 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,4 +1,6 @@ -| 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' | +| SpringExporterUnsafeDeserialization.java:14:24:14:47 | unsafeRmiServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeRmiServiceExporter' | +| SpringExporterUnsafeDeserialization.java:24:28:24:55 | unsafeHessianServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeHessianServiceExporter' | +| SpringExporterUnsafeDeserialization.java:32:32:32:63 | unsafeHttpInvokerServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeHttpInvokerServiceExporter' | +| SpringExporterUnsafeDeserialization.java:40:41:40:88 | unsafeCustomeRemoteInvocationSerializingExporter | Unsafe deserialization in a Spring exporter bean '/unsafeCustomeRemoteInvocationSerializingExporter' | +| SpringExporterUnsafeDeserialization.java:56:32:56:63 | unsafeHttpInvokerServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeHttpInvokerServiceExporter' | +| SpringExporterUnsafeDeserialization.java:68:32:68:63 | unsafeHttpInvokerServiceExporter | Unsafe deserialization in a Spring exporter bean '/unsafeHttpInvokerServiceExporter' | 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 index edcb3668557b..ec7a06f8bfcf 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInXMLConfiguration.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-502/UnsafeSpringExporterInXMLConfiguration.expected @@ -1,2 +1,4 @@ | 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' | +| beans.xml:20:5:24:12 | org.springframework.remoting.rmi.RmiServiceExporter | Unsafe deserialization in a Spring exporter bean 'org.springframework.remoting.rmi.RmiServiceExporter' | +| beans.xml:26:5:29:12 | org.springframework.remoting.caucho.HessianServiceExporter | Unsafe deserialization in a Spring exporter bean 'org.springframework.remoting.caucho.HessianServiceExporter' | 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 index bdb4e5fa6515..fbb936d901db 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-502/beans.xml +++ b/java/ql/test/experimental/query-tests/security/CWE-502/beans.xml @@ -16,4 +16,15 @@ + + + + + + + + + + + diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/caucho/HessianExporter.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/caucho/HessianExporter.java new file mode 100644 index 000000000000..a3e81497850c --- /dev/null +++ b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/caucho/HessianExporter.java @@ -0,0 +1,8 @@ +package org.springframework.remoting.caucho; + +public class HessianExporter { + + 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/caucho/HessianServiceExporter.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/caucho/HessianServiceExporter.java new file mode 100644 index 000000000000..9acc2093032c --- /dev/null +++ b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/caucho/HessianServiceExporter.java @@ -0,0 +1,3 @@ +package org.springframework.remoting.caucho; + +public class HessianServiceExporter extends HessianExporter {} \ No newline at end of file diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RmiBasedExporter.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RmiBasedExporter.java new file mode 100644 index 000000000000..80135404e157 --- /dev/null +++ b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RmiBasedExporter.java @@ -0,0 +1,12 @@ +package org.springframework.remoting.rmi; + +public abstract class RmiBasedExporter { + + public void setService(Object service) {} + + public void setServiceInterface(Class clazz) {} + + public void setServiceName(String name) {} + + public void setRegistryPort(int port) {} +} \ No newline at end of file diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RmiServiceExporter.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RmiServiceExporter.java new file mode 100644 index 000000000000..f5d5f8856356 --- /dev/null +++ b/java/ql/test/stubs/springframework-5.2.3/org/springframework/remoting/rmi/RmiServiceExporter.java @@ -0,0 +1,3 @@ +package org.springframework.remoting.rmi; + +public class RmiServiceExporter extends RmiBasedExporter {} \ No newline at end of file From 0dfb869c5b74cd8547443b24c0cc592df9e9438e Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 23 Jun 2021 13:23:54 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Chris Smowton --- .../Security/CWE/CWE-502/UnsafeSpringExporterQuery.inc.qhelp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 index f7acc417dc84..977a7fac3d95 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterQuery.inc.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-502/UnsafeSpringExporterQuery.inc.qhelp @@ -14,11 +14,10 @@ it results in remote code execution in the worst case.

-Here are examples of unsafe exporters: HttpInvokerServiceExporter, +Examples of unsafe exporters include: HttpInvokerServiceExporter, SimpleHttpInvokerServiceExporter, RmiServiceExporter, HessianServiceExporter.

-

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. @@ -34,4 +33,4 @@ consider using global deserialization filters introduced in JEP 290.

- \ No newline at end of file +