Merge branch 'main' into java-followMsg

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 { }
@@ -86,6 +87,8 @@ private class GetResourceSink extends UnsafeUrlForwardSink {
   GetResourceSink() {
     sinkNode(this, "open-url")
     or
+    sinkNode(this, "get-resource")
+    or
     exists(MethodAccess ma |
       (
         ma.getMethod() instanceof GetServletResourceAsStreamMethod or
@@ -99,6 +102,16 @@ private class GetResourceSink extends UnsafeUrlForwardSink {
   }
 }
 
+/** A sink for methods that load Spring resources. */
+private class SpringResourceSink extends UnsafeUrlForwardSink {
+  SpringResourceSink() {
+    exists(MethodAccess ma |
+      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 +188,25 @@ private class FilePathFlowStep extends SummaryModelCsv {
       ]
   }
 }
+
+/** Taint models 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;manual",
+        "org.springframework.core.io;ResourceLoader;true;getResource;;;Argument[0];ReturnValue;taint;manual",
+        "org.springframework.core.io;Resource;true;createRelative;;;Argument[0];ReturnValue;taint;manual"
+      ]
+  }
+}
+
+/** Sink models for methods that load Spring resources. */
+private class SpringResourceCsvSink extends SinkModelCsv {
+  override predicate row(string row) {
+    row =
+      // Get spring resource
+      "org.springframework.core.io;ClassPathResource;true;" +
+        ["getFilename", "getPath", "getURL", "resolveURL"] + ";;;Argument[-1];get-resource;manual"
+  }
+}

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

@@ -0,0 +1,22 @@
+/**
+ * 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 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") }
+}
+
+/**
+ * A method declared in `org.springframework.util.ResourceUtils` that loads Spring resources.
+ */
+class GetResourceUtilsMethod extends Method {
+  GetResourceUtilsMethod() {
+    this.getDeclaringType().getASupertype*() instanceof ResourceUtils and
+    this.hasName(["extractArchiveURL", "extractJarFileURL", "getFile", "getURL"])
+  }
+}