Refactored HttpsUrls.ql

Author: atorralba

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

@@ -52,11 +52,20 @@ class UriCreation extends Call {
   /**
    * Gets the host argument of the newly created URI. In the case where the
    * host is specified separately, this is only the host. In the case where the
-   * uri is parsed from an input string, such as in
+   * URI is parsed from an input string, such as in
    * `URI("http://foo.com/mypath")`, this is the entire argument passed in,
    * that is `"http://foo.com/mypath"`.
    */
-  Expr getHostArg() { none() }
+  abstract Expr getHostArg();
+
+  /**
+   * Gets the scheme argument of the newly created URI. In the case where the
+   * scheme is specified separately, this is only the scheme. In the case where the
+   * URI is parsed from an input string, such as in
+   * `URI("http://foo.com/mypath")`, this is the entire argument passed in,
+   * that is `"http://foo.com/mypath"`.
+   */
+  abstract Expr getSchemeArg();
 }
 
 /** A `java.net.URI` constructor call. */
@@ -74,13 +83,17 @@ class UriConstructorCall extends ClassInstanceExpr, UriCreation {
     //    String fragment)
     result = this.getArgument(2) and this.getNumArgument() = 7
   }
+
+  override Expr getSchemeArg() { result = this.getArgument(0) }
 }
 
 /** A call to `java.net.URI::create`. */
 class UriCreate extends UriCreation {
   UriCreate() { this.getCallee().hasName("create") }
 
   override Expr getHostArg() { result = this.getArgument(0) }
+
+  override Expr getSchemeArg() { result = this.getArgument(0) }
 }
 
 /** A `java.net.URL` constructor call. */
@@ -105,7 +118,7 @@ class UrlConstructorCall extends ClassInstanceExpr {
   }
 
   /** Gets the argument that corresponds to the protocol of the URL. */
-  Expr protocolArg() {
+  Expr getProtocolArg() {
     // In all cases except where the first parameter is a URL, the argument
     // containing the protocol is the first one, otherwise it is the second.
     if this.getConstructor().getParameterType(0) instanceof TypeUrl

java/ql/lib/semmle/code/java/security/HttpsUrls.qll

@@ -0,0 +1,89 @@
+import java
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.dataflow.TaintTracking
+private import semmle.code.java.frameworks.ApacheHttp
+private import semmle.code.java.frameworks.Networking
+
+/**
+ * String of HTTP URLs not in private domains.
+ */
+class HttpStringLiteral extends StringLiteral {
+  HttpStringLiteral() {
+    exists(string s | this.getRepresentedString() = s |
+      s = "http"
+      or
+      s.matches("http://%") and
+      not s.substring(7, s.length()) instanceof PrivateHostName and
+      not TaintTracking::localExprTaint(any(StringLiteral p |
+          p.getRepresentedString() instanceof PrivateHostName
+        ), this.getParent*())
+    )
+  }
+}
+
+/**
+ * A sink that represents a URL opening method call, such as a call to `java.net.URL.openConnection()`.
+ */
+abstract class UrlOpenSink extends DataFlow::Node { }
+
+private class DefaultUrlOpenSink extends UrlOpenSink {
+  DefaultUrlOpenSink() { sinkNode(this, "open-url") }
+}
+
+/**
+ * A unit class for adding additional taint steps.
+ *
+ * Extend this class to add additional taint steps that should apply
+ * to configurations working with HTTP URLs.
+ */
+class HttpUrlsAdditionalTaintStep extends Unit {
+  /**
+   * Holds if the step from `node1` to `node2` should be considered a taint
+   * step for taint tracking configurations working with HTTP URLs.
+   */
+  abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
+}
+
+private class DefaultHttpUrlAdditionalTaintStep extends HttpUrlsAdditionalTaintStep {
+  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+    apacheHttpRequestStep(n1, n2) or
+    createUriStep(n1, n2) or
+    createUrlStep(n1, n2) or
+    urlOpenStep(n1, n2)
+  }
+}
+
+/** Constructor of `ApacheHttpRequest` */
+private predicate apacheHttpRequestStep(DataFlow::Node node1, DataFlow::Node node2) {
+  exists(ConstructorCall cc |
+    cc.getConstructedType() instanceof ApacheHttpRequest and
+    node2.asExpr() = cc and
+    cc.getAnArgument() = node1.asExpr()
+  )
+}
+
+/** `URI` methods */
+private predicate createUriStep(DataFlow::Node node1, DataFlow::Node node2) {
+  exists(UriConstructorCall cc |
+    cc.getSchemeArg() = node1.asExpr() and
+    node2.asExpr() = cc
+  )
+}
+
+/** Constructors of `URL` */
+private predicate createUrlStep(DataFlow::Node node1, DataFlow::Node node2) {
+  exists(UrlConstructorCall cc |
+    cc.getProtocolArg() = node1.asExpr() and
+    node2.asExpr() = cc
+  )
+}
+
+/** Method call of `HttpURLOpenMethod` */
+private predicate urlOpenStep(DataFlow::Node node1, DataFlow::Node node2) {
+  exists(MethodAccess ma |
+    ma.getMethod() instanceof UrlOpenConnectionMethod and
+    node1.asExpr() = ma.getQualifier() and
+    ma = node2.asExpr()
+  )
+}

java/ql/lib/semmle/code/java/security/HttpsUrlsQuery.qll

@@ -0,0 +1,25 @@
+/** Provides taint tracking configurations to be used in HTTPS URLs queries. */
+
+import java
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.frameworks.Networking
+import semmle.code.java.security.HttpsUrls
+
+/**
+ * A taint tracking configuration for HTTP connections.
+ */
+class HttpStringToUrlOpenMethodFlowConfig extends TaintTracking::Configuration {
+  HttpStringToUrlOpenMethodFlowConfig() { this = "HttpStringToUrlOpenMethodFlowConfig" }
+
+  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof HttpStringLiteral }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof UrlOpenSink }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(HttpUrlsAdditionalTaintStep c).step(node1, node2)
+  }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
+  }
+}

java/ql/src/Security/CWE/CWE-319/HttpsUrls.ql

@@ -11,54 +11,10 @@
  */
 
 import java
-import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.frameworks.Networking
+import semmle.code.java.security.HttpsUrlsQuery
 import DataFlow::PathGraph
-private import semmle.code.java.dataflow.ExternalFlow
 
-class HttpString extends StringLiteral {
-  HttpString() {
-    // Avoid matching "https" here.
-    exists(string s | this.getRepresentedString() = s |
-      (
-        // Either the literal "http", ...
-        s = "http"
-        or
-        // ... or the beginning of a http URL.
-        s.matches("http://%")
-      ) and
-      not s.matches("%/localhost%")
-    )
-  }
-}
-
-class HttpStringToUrlOpenMethodFlowConfig extends TaintTracking::Configuration {
-  HttpStringToUrlOpenMethodFlowConfig() { this = "HttpsUrls::HttpStringToUrlOpenMethodFlowConfig" }
-
-  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof HttpString }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof UrlOpenSink }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-    exists(UrlConstructorCall u |
-      node1.asExpr() = u.protocolArg() and
-      node2.asExpr() = u
-    )
-  }
-
-  override predicate isSanitizer(DataFlow::Node node) {
-    node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
-  }
-}
-
-/**
- * A sink that represents a URL opening method call, such as a call to `java.net.URL.openConnection()`.
- */
-private class UrlOpenSink extends DataFlow::Node {
-  UrlOpenSink() { sinkNode(this, "open-url") }
-}
-
-from DataFlow::PathNode source, DataFlow::PathNode sink, MethodAccess m, HttpString s
+from DataFlow::PathNode source, DataFlow::PathNode sink, MethodAccess m, HttpStringLiteral s
 where
   source.getNode().asExpr() = s and
   sink.getNode().asExpr() = m.getQualifier() and