Merge pull request github#2899 from p-/cwe-036

Java: Calling openStream on URLs created from remote source can lead to file disclosure

Author: aschackmull

java/ql/src/experimental/Security/CWE/CWE-036/OpenStream.java

@@ -0,0 +1,8 @@
+public class TestServlet extends HttpServlet {
+    protected void doGet(HttpServletRequest request, HttpServletResponse response)
+    throws ServletException, IOException {
+        // BAD: a URL from a remote source is opened with URL#openStream()
+        URL url = new URL(request.getParameter("url"));
+        InputStream inputStream = new URL(url).openStream();
+    }
+}

java/ql/src/experimental/Security/CWE/CWE-036/OpenStream.qhelp

@@ -0,0 +1,33 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Calling <code>openStream</code> on URLs created from remote source can lead to local file disclosure.</p>
+<p>If <code>openStream</code> is called on a <code>java.net.___URL</code>, that was created from a remote source,
+an attacker can try to pass absolute URLs starting with <code>file://</code> or <code>jar://</code> to access
+local resources in addition to remote ones.</p>
+</overview>
+
+<recommendation>
+<p>When you construct a URL using <code>java.net.___URL</code> from a remote source,
+don't call <code>openStream</code> on it. Instead, use an HTTP Client to fetch the URL and access its content.
+You should also validate the URL to check that it uses the correct protocol and host combination.</p>
+</recommendation>
+
+<example>
+<p>The following example shows an URL that is constructed from a request parameter. Afterwards <code>openStream</code>
+is called on the URL, potentially leading to a local file access.</p>
+<sample src="OpenStream.java" />
+</example>
+
+<references>
+<li>Java Platform, Standard Edition 11, API Specification: 
+<a href="https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/URL.html">
+Class URL</a>.
+</li>
+<!--  LocalWords:  CWE -->
+</references>
+
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-036/OpenStream.ql

@@ -0,0 +1,55 @@
+/**
+ * @name openStream called on URLs created from remote source
+ * @description Calling openStream on URLs created from remote source
+ * can lead to local file disclosure.
+ * @kind path-problem
+ */
+
+import java
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.dataflow.FlowSources
+import DataFlow::PathGraph
+
+class URLConstructor extends ClassInstanceExpr {
+  URLConstructor() { this.getConstructor().getDeclaringType() instanceof TypeUrl }
+
+  Expr stringArg() {
+    // Query only in URL's that were constructed by calling the single parameter string constructor.
+    this.getConstructor().getNumberOfParameters() = 1 and
+    this.getConstructor().getParameter(0).getType() instanceof TypeString and
+    result = this.getArgument(0)
+  }
+}
+
+class URLOpenStreamMethod extends Method {
+  URLOpenStreamMethod() {
+    this.getDeclaringType() instanceof TypeUrl and
+    this.getName() = "openStream"
+  }
+}
+
+class RemoteURLToOpenStreamFlowConfig extends TaintTracking::Configuration {
+  RemoteURLToOpenStreamFlowConfig() { this = "OpenStream::RemoteURLToOpenStreamFlowConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(MethodAccess m |
+      sink.asExpr() = m.getQualifier() and m.getMethod() instanceof URLOpenStreamMethod
+    )
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    exists(URLConstructor u |
+      node1.asExpr() = u.stringArg() and
+      node2.asExpr() = u
+    )
+  }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, MethodAccess call
+where
+  sink.getNode().asExpr() = call.getQualifier() and
+  any(RemoteURLToOpenStreamFlowConfig c).hasFlowPath(source, sink)
+select call, source, sink,
+  "URL on which openStream is called may have been constructed from remote source"

java/ql/src/semmle/code/java/frameworks/Networking.qll

@@ -12,6 +12,10 @@ class TypeSocket extends RefType {
   TypeSocket() { hasQualifiedName("java.net", "Socket") }
 }
 
+class TypeUrl extends RefType {
+  TypeUrl() { hasQualifiedName("java.net", "URL") }
+}
+
 class URLConnectionGetInputStreamMethod extends Method {
   URLConnectionGetInputStreamMethod() {
     getDeclaringType() instanceof TypeUrlConnection and