Java: copy files from experimental

2026-04-29 18:55:14 +02:00 · 2023-11-20 15:12:33 -05:00
parent 67b3670d06
commit 0d38a9625e
17 changed files with 1320 additions and 0 deletions
--- a/java/ql/src/Security/CWE/CWE-552/UnsafeLoadSpringResource.java
+++ b/java/ql/src/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);
+	}
+}
--- a/java/ql/src/Security/CWE/CWE-552/UnsafeResourceGet.java
+++ b/java/ql/src/Security/CWE/CWE-552/UnsafeResourceGet.java
@@ -0,0 +1,18 @@
+// BAD: no URI validation
+URL url = request.getServletContext().getResource(requestUrl);
+url = getClass().getResource(requestUrl);
+InputStream in = url.openStream();
+
+InputStream in = request.getServletContext().getResourceAsStream(requestPath);
+in = getClass().getClassLoader().getResourceAsStream(requestPath);
+
+// GOOD: check for a trusted prefix, ensuring path traversal is not used to erase that prefix:
+// (alternatively use `Path.normalize` instead of checking for `..`)
+if (!requestPath.contains("..") && requestPath.startsWith("/trusted")) {
+	InputStream in = request.getServletContext().getResourceAsStream(requestPath);
+}
+
+Path path = Paths.get(requestUrl).normalize().toRealPath();
+if (path.startsWith("/trusted")) {
+	URL url = request.getServletContext().getResource(path.toString());
+}
--- a/java/ql/src/Security/CWE/CWE-552/UnsafeServletRequestDispatch.java
+++ b/java/ql/src/Security/CWE/CWE-552/UnsafeServletRequestDispatch.java
@@ -0,0 +1,11 @@
+// BAD: no URI validation
+String returnURL = request.getParameter("returnURL");
+RequestDispatcher rd = sc.getRequestDispatcher(returnURL);
+rd.forward(request, response);
+
+// GOOD: check for a trusted prefix, ensuring path traversal is not used to erase that prefix:
+// (alternatively use `Path.normalize` instead of checking for `..`)
+if (!returnURL.contains("..") && returnURL.hasPrefix("/pages")) { ... }
+// Also GOOD: check for a forbidden prefix, ensuring URL-encoding is not used to evade the check:
+// (alternatively use `URLDecoder.decode` before `hasPrefix`)
+if (returnURL.hasPrefix("/internal") && !returnURL.contains("%")) { ... }
--- a/java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.java
+++ b/java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.java
@@ -0,0 +1,38 @@
+import java.io.IOException;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.springframework.stereotype.Controller;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.servlet.ModelAndView;
+
+@Controller
+public class UnsafeUrlForward {
+
+	@GetMapping("/bad1")
+	public ModelAndView bad1(String url) {
+		return new ModelAndView(url);
+	}
+
+	@GetMapping("/bad2")
+	public void bad2(String url, HttpServletRequest request, HttpServletResponse response) {
+		try {
+			request.getRequestDispatcher("/WEB-INF/jsp/" + url + ".jsp").include(request, response);
+		} catch (ServletException e) {
+			e.printStackTrace();
+		} catch (IOException e) {
+			e.printStackTrace();
+		}
+	}
+
+	@GetMapping("/good1")
+	public void good1(String url, HttpServletRequest request, HttpServletResponse response) {
+		try {
+			request.getRequestDispatcher("/index.jsp?token=" + url).forward(request, response);
+		} catch (ServletException e) {
+			e.printStackTrace();
+		} catch (IOException e) {
+			e.printStackTrace();
+		}
+	}
+}
--- a/java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.qhelp
+++ b/java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.qhelp
@@ -0,0 +1,70 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p>Constructing a server-side redirect path with user input could allow an attacker to download application binaries
+(including application classes or jar files) or view arbitrary files within protected directories.</p>
+
+</overview>
+<recommendation>
+
+<p>Unsanitized user provided data must not be used to construct the path for URL forwarding. In order to prevent
+untrusted URL forwarding, it is recommended to avoid concatenating user input directly into the forwarding URL.
+Instead, user input should be checked against allowed (e.g., must come within <code>user_content/</code>) or disallowed
+(e.g. must not come within <code>/internal</code>) paths, ensuring that neither path traversal using <code>../</code>
+or URL encoding are used to evade these checks.
+</p>
+
+</recommendation>
+<example>
+
+<p>The following examples show the bad case and the good case respectively.
+The <code>bad</code> methods show an HTTP request parameter being used directly in a URL forward
+without validating the input, which may cause file leakage. In the <code>good1</code> method,
+ordinary forwarding requests are shown, which will not cause file leakage.
+</p>
+
+<sample src="UnsafeUrlForward.java" />
+
+<p>The following examples show an HTTP request parameter or request path being used directly in a
+request dispatcher of Java EE 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="UnsafeServletRequestDispatch.java" />
+
+<p>The following examples show an HTTP request parameter or request path being used directly to
+retrieve a resource of a Java EE 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="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:
+  <a href="https://vulncat.fortify.com/en/detail?id=desc.dataflow.java.file_disclosure_spring">Unsafe Url Forward</a>.
+</li>
+<li>Jakarta Javadoc:
+  <a href="https://jakarta.ee/specifications/webprofile/9/apidocs/jakarta/servlet/servletrequest#getRequestDispatcher-java.lang.String-">Security vulnerability with unsafe usage of RequestDispatcher</a>.
+</li>
+<li>Micro Focus:
+  <a href="https://vulncat.fortify.com/en/detail?id=desc.dataflow.java.file_disclosure_j2ee">File Disclosure: J2EE</a>
+</li>
+<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>
--- a/java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.ql
+++ b/java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.ql
@@ -0,0 +1,63 @@
+/**
+ * @name Unsafe URL forward or include from a remote source
+ * @description URL forward or include based on unvalidated user-input
+ *              may cause file information disclosure.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/unsafe-url-forward-include
+ * @tags security
+ *       external/cwe-552
+ */
+
+import java
+import UnsafeUrlForward
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.TaintTracking
+import experimental.semmle.code.java.frameworks.Jsf
+import semmle.code.java.security.PathSanitizer
+import UnsafeUrlForwardFlow::PathGraph
+
+module UnsafeUrlForwardFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source instanceof ThreatModelFlowSource and
+    not exists(MethodCall ma, Method m | ma.getMethod() = m |
+      (
+        m instanceof HttpServletRequestGetRequestUriMethod or
+        m instanceof HttpServletRequestGetRequestUrlMethod or
+        m instanceof HttpServletRequestGetPathMethod
+      ) and
+      ma = source.asExpr()
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeUrlForwardSink }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node instanceof UnsafeUrlForwardSanitizer or
+    node instanceof PathInjectionSanitizer
+  }
+
+  DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
+
+  predicate isAdditionalFlowStep(DataFlow::Node prev, DataFlow::Node succ) {
+    exists(MethodCall ma |
+      (
+        ma.getMethod() instanceof GetServletResourceMethod or
+        ma.getMethod() instanceof GetFacesResourceMethod or
+        ma.getMethod() instanceof GetClassResourceMethod or
+        ma.getMethod() instanceof GetClassLoaderResourceMethod or
+        ma.getMethod() instanceof GetWildflyResourceMethod
+      ) and
+      ma.getArgument(0) = prev.asExpr() and
+      ma = succ.asExpr()
+    )
+  }
+}
+
+module UnsafeUrlForwardFlow = TaintTracking::Global<UnsafeUrlForwardFlowConfig>;
+
+from UnsafeUrlForwardFlow::PathNode source, UnsafeUrlForwardFlow::PathNode sink
+where UnsafeUrlForwardFlow::flowPath(source, sink)
+select sink.getNode(), source, sink, "Potentially untrusted URL forward due to $@.",
+  source.getNode(), "user-provided value"
--- a/java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.qll
+++ b/java/ql/src/Security/CWE/CWE-552/UnsafeUrlForward.qll
@@ -0,0 +1,163 @@
+import java
+private import experimental.semmle.code.java.frameworks.Jsf
+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 { }
+
+/** A sanitizer for unsafe URL forward vulnerabilities. */
+abstract class UnsafeUrlForwardSanitizer extends DataFlow::Node { }
+
+/** An argument to `getRequestDispatcher`. */
+private class RequestDispatcherSink extends UnsafeUrlForwardSink {
+  RequestDispatcherSink() {
+    exists(MethodCall ma |
+      ma.getMethod() instanceof GetRequestDispatcherMethod and
+      ma.getArgument(0) = this.asExpr()
+    )
+  }
+}
+
+/** The `getResource` method of `Class`. */
+class GetClassResourceMethod extends Method {
+  GetClassResourceMethod() {
+    this.getDeclaringType() instanceof TypeClass and
+    this.hasName("getResource")
+  }
+}
+
+/** The `getResourceAsStream` method of `Class`. */
+class GetClassResourceAsStreamMethod extends Method {
+  GetClassResourceAsStreamMethod() {
+    this.getDeclaringType() instanceof TypeClass and
+    this.hasName("getResourceAsStream")
+  }
+}
+
+/** The `getResource` method of `ClassLoader`. */
+class GetClassLoaderResourceMethod extends Method {
+  GetClassLoaderResourceMethod() {
+    this.getDeclaringType() instanceof ClassLoaderClass and
+    this.hasName("getResource")
+  }
+}
+
+/** The `getResourceAsStream` method of `ClassLoader`. */
+class GetClassLoaderResourceAsStreamMethod extends Method {
+  GetClassLoaderResourceAsStreamMethod() {
+    this.getDeclaringType() instanceof ClassLoaderClass and
+    this.hasName("getResourceAsStream")
+  }
+}
+
+/** The JBoss class `FileResourceManager`. */
+class FileResourceManager extends RefType {
+  FileResourceManager() {
+    this.hasQualifiedName("io.undertow.server.handlers.resource", "FileResourceManager")
+  }
+}
+
+/** The JBoss method `getResource` of `FileResourceManager`. */
+class GetWildflyResourceMethod extends Method {
+  GetWildflyResourceMethod() {
+    this.getDeclaringType().getASupertype*() instanceof FileResourceManager and
+    this.hasName("getResource")
+  }
+}
+
+/** The JBoss class `VirtualFile`. */
+class VirtualFile extends RefType {
+  VirtualFile() { this.hasQualifiedName("org.jboss.vfs", "VirtualFile") }
+}
+
+/** The JBoss method `getChild` of `FileResourceManager`. */
+class GetVirtualFileChildMethod extends Method {
+  GetVirtualFileChildMethod() {
+    this.getDeclaringType().getASupertype*() instanceof VirtualFile and
+    this.hasName("getChild")
+  }
+}
+
+/** An argument to `getResource()` or `getResourceAsStream()`. */
+private class GetResourceSink extends UnsafeUrlForwardSink {
+  GetResourceSink() {
+    sinkNode(this, "request-forgery")
+    or
+    sinkNode(this, "get-resource")
+    or
+    exists(MethodCall ma |
+      (
+        ma.getMethod() instanceof GetServletResourceAsStreamMethod or
+        ma.getMethod() instanceof GetFacesResourceAsStreamMethod or
+        ma.getMethod() instanceof GetClassResourceAsStreamMethod or
+        ma.getMethod() instanceof GetClassLoaderResourceAsStreamMethod or
+        ma.getMethod() instanceof GetVirtualFileChildMethod
+      ) and
+      ma.getArgument(0) = this.asExpr()
+    )
+  }
+}
+
+/** A sink for methods that load Spring resources. */
+private class SpringResourceSink extends UnsafeUrlForwardSink {
+  SpringResourceSink() {
+    exists(MethodCall 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() {
+    exists(ClassInstanceExpr cie |
+      cie.getConstructedType() instanceof ModelAndView and
+      cie.getArgument(0) = this.asExpr()
+    )
+    or
+    exists(SpringModelAndViewSetViewNameCall smavsvnc | smavsvnc.getArgument(0) = this.asExpr())
+  }
+}
+
+private class PrimitiveSanitizer extends UnsafeUrlForwardSanitizer {
+  PrimitiveSanitizer() {
+    this.getType() instanceof PrimitiveType or
+    this.getType() instanceof BoxedType or
+    this.getType() instanceof NumberType
+  }
+}
+
+private class SanitizingPrefix extends InterestingPrefix {
+  SanitizingPrefix() {
+    not this.getStringValue().matches("/WEB-INF/%") and
+    not this.getStringValue() = "forward:"
+  }
+
+  override int getOffset() { result = 0 }
+}
+
+private class FollowsSanitizingPrefix extends UnsafeUrlForwardSanitizer {
+  FollowsSanitizingPrefix() { this.asExpr() = any(SanitizingPrefix fp).getAnAppendedExpression() }
+}
+
+private class ForwardPrefix extends InterestingPrefix {
+  ForwardPrefix() { this.getStringValue() = "forward:" }
+
+  override int getOffset() { result = 0 }
+}
+
+/**
+ * An expression appended (perhaps indirectly) to `"forward:"`, and which
+ * is reachable from a Spring entry point.
+ */
+private class SpringUrlForwardSink extends UnsafeUrlForwardSink {
+  SpringUrlForwardSink() {
+    any(SpringRequestMappingMethod sqmm).polyCalls*(this.getEnclosingCallable()) and
+    this.asExpr() = any(ForwardPrefix fp).getAnAppendedExpression()
+  }
+}