Query to detect unsafe resource loading in Java Spring applications

Author: luchua-bc

java/ql/src/experimental/Security/CWE/CWE-552/UnsafeLoadSpringResource.java

@@ -0,0 +1,21 @@
+//BAD: no path validation in Spring resource loading
+@GetMapping("/file")
+public String getFileContent(@RequestParam(name="fileName") String fileName) {
+	ClassPathResource clr = new ClassPathResource(fileName);
+
+	File file = ResourceUtils.getFile(fileName);
+
+	Resource resource = resourceLoader.getResource(fileName);
+}
+
+//GOOD: check for a trusted prefix, ensuring path traversal is not used to erase that prefix in Spring resource loading:
+@GetMapping("/file")
+public String getFileContent(@RequestParam(name="fileName") String fileName) {
+	if (!fileName.contains("..") && fileName.hasPrefix("/public-content")) {
+		ClassPathResource clr = new ClassPathResource(fileName);
+
+		File file = ResourceUtils.getFile(fileName);
+
+		Resource resource = resourceLoader.getResource(fileName);
+	}
+}

java/ql/src/experimental/Security/CWE/CWE-552/UnsafeUrlForward.qhelp

@@ -43,6 +43,12 @@ file exposure attacks. It also shows how to remedy the problem by validating the
 
 <sample src="UnsafeResourceGet.java" />
 
+<p>The following examples show an HTTP request parameter being used directly to retrieve a resource
+  of a Java Spring application without validating the input, which allows sensitive file exposure
+  attacks. It also shows how to remedy the problem by validating the user input.
+  </p>
+
+  <sample src="UnsafeLoadSpringResource.java" />
 </example>
 <references>
 <li>File Disclosure:
@@ -57,5 +63,8 @@ file exposure attacks. It also shows how to remedy the problem by validating the
 <li>CVE-2015-5174:
   <a href="https://vuldb.com/?id.81084">Apache Tomcat 6.0/7.0/8.0/9.0 Servletcontext getResource/getResourceAsStream/getResourcePaths Path Traversal</a>
 </li>
+<li>CVE-2019-3799:
+  <a href="https://github.com/mpgn/CVE-2019-3799">CVE-2019-3799 - Spring-Cloud-Config-Server Directory Traversal &lt; 2.1.2, 2.0.4, 1.4.6</a>
+</li>
 </references>
 </qhelp>

java/ql/src/experimental/Security/CWE/CWE-552/UnsafeUrlForward.ql

@@ -1,11 +1,11 @@
 /**
- * @name Unsafe URL forward or dispatch from remote source
- * @description URL forward or dispatch based on unvalidated user-input
+ * @name Unsafe URL forward, dispatch, or load from remote source
+ * @description URL forward, dispatch, or load based on unvalidated user-input
  *              may cause file information disclosure.
  * @kind path-problem
  * @problem.severity error
  * @precision high
- * @id java/unsafe-url-forward-dispatch
+ * @id java/unsafe-url-forward-dispatch-load
  * @tags security
  *       external/cwe-552
  */

java/ql/src/experimental/Security/CWE/CWE-552/UnsafeUrlForward.qll

@@ -4,6 +4,7 @@ private import semmle.code.java.dataflow.ExternalFlow
 private import semmle.code.java.dataflow.FlowSources
 private import semmle.code.java.dataflow.StringPrefixes
 private import semmle.code.java.frameworks.javaee.ejb.EJBRestrictions
+private import experimental.semmle.code.java.frameworks.SpringResource
 
 /** A sink for unsafe URL forward vulnerabilities. */
 abstract class UnsafeUrlForwardSink extends DataFlow::Node { }
@@ -99,6 +100,22 @@ private class GetResourceSink extends UnsafeUrlForwardSink {
   }
 }
 
+/** Sink of Spring resource loading. */
+private class SpringResourceSink extends UnsafeUrlForwardSink {
+  SpringResourceSink() {
+    exists(MethodAccess ma |
+      (
+        ma.getMethod() instanceof GetClassPathResourceInputStreamMethod or
+        ma.getMethod() instanceof GetResourceMethod
+      ) and
+      ma.getQualifier() = this.asExpr()
+      or
+      ma.getMethod() instanceof GetResourceUtilsMethod and
+      ma.getArgument(0) = this.asExpr()
+    )
+  }
+}
+
 /** An argument to `new ModelAndView` or `ModelAndView.setViewName`. */
 private class SpringModelAndViewSink extends UnsafeUrlForwardSink {
   SpringModelAndViewSink() {
@@ -175,3 +192,17 @@ private class FilePathFlowStep extends SummaryModelCsv {
       ]
   }
 }
+
+/** Taint model related to resource loading in Spring. */
+private class LoadSpringResourceFlowStep extends SummaryModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "org.springframework.core.io;ClassPathResource;false;ClassPathResource;;;Argument[0];Argument[-1];taint",
+        "org.springframework.core.io;ClassPathResource;true;" +
+          ["getFilename", "getPath", "getURL", "resolveURL"] + ";;;Argument[-1];ReturnValue;value",
+        "org.springframework.core.io;ResourceLoader;true;getResource;;;Argument[0];ReturnValue;taint",
+        "org.springframework.core.io;Resource;true;createRelative;;;Argument[0];ReturnValue;taint"
+      ]
+  }
+}

java/ql/src/experimental/semmle/code/java/frameworks/SpringResource.qll

@@ -0,0 +1,62 @@
+/**
+ * Provides classes for working with resource loading in Spring.
+ */
+
+import java
+private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.dataflow.FlowSources
+
+/** A class for class path resources in the Spring framework. */
+class ClassPathResource extends Class {
+  ClassPathResource() { this.hasQualifiedName("org.springframework.core.io", "ClassPathResource") }
+}
+
+/** An interface for objects that are sources for an InputStream in the Spring framework. */
+class InputStreamResource extends RefType {
+  InputStreamResource() {
+    this.hasQualifiedName("org.springframework.core.io", "InputStreamSource")
+  }
+}
+
+/** An interface that abstracts from the underlying resource, such as a file or class path resource in the Spring framework. */
+class Resource extends RefType {
+  Resource() { this.hasQualifiedName("org.springframework.core.io", "Resource") }
+}
+
+/** A utility class for resolving resource locations to files in the file system in the Spring framework. */
+class ResourceUtils extends Class {
+  ResourceUtils() { this.hasQualifiedName("org.springframework.util", "ResourceUtils") }
+}
+
+/**
+ * The method `getInputStream()` declared in Spring `ClassPathResource`.
+ */
+class GetClassPathResourceInputStreamMethod extends Method {
+  GetClassPathResourceInputStreamMethod() {
+    this.getDeclaringType().getASupertype*() instanceof ClassPathResource and
+    this.hasName("getInputStream")
+  }
+}
+
+/**
+ * Resource loading method declared in Spring `Resource` with `getInputStream` inherited from the parent interface.
+ */
+class GetResourceMethod extends Method {
+  GetResourceMethod() {
+    (
+      this.getDeclaringType() instanceof InputStreamResource or
+      this.getDeclaringType() instanceof Resource
+    ) and
+    this.hasName(["getFile", "getFilename", "getInputStream", "getURI", "getURL"])
+  }
+}
+
+/**
+ * Resource loading method declared in Spring `ResourceUtils`.
+ */
+class GetResourceUtilsMethod extends Method {
+  GetResourceUtilsMethod() {
+    this.getDeclaringType().getASupertype*() instanceof ResourceUtils and
+    this.hasName(["extractArchiveURL", "extractJarFileURL", "getFile", "getURL", "toURI"])
+  }
+}