Convert SSRF sinks into url-open CSV sinks

I also drop the previous approach of taint-tracking through various builder objects in favour of assuming that a URI set in a request-builder object is highly likely to end up requested in some way or another.

This will cause the `java/non-https-url` query to pick the new sinks up too, and fixes a Spring case that had never worked but went unnoticed until now.

java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll

@@ -82,6 +82,8 @@ private module Frameworks {
   private import semmle.code.java.frameworks.guava.Guava
   private import semmle.code.java.frameworks.jackson.JacksonSerializability
   private import semmle.code.java.frameworks.JaxWS
+  private import semmle.code.java.frameworks.spring.SpringHttp
+  private import semmle.code.java.frameworks.spring.SpringWebClient
   private import semmle.code.java.security.ResponseSplitting
   private import semmle.code.java.security.InformationLeak
   private import semmle.code.java.security.XSS
@@ -209,6 +211,8 @@ private predicate sinkModelCsv(string row) {
       // Open URL
       "java.net;URL;false;openConnection;;;Argument[-1];open-url",
       "java.net;URL;false;openStream;;;Argument[-1];open-url",
+      "java.net.http;HttpRequest;false;newBuilder;;;Argument[0];open-url",
+      "java.net.http;HttpRequest$Builder;false;uri;;;Argument[0];open-url",
       // Create file
       "java.io;FileOutputStream;false;FileOutputStream;;;Argument[0];create-file",
       "java.io;RandomAccessFile;false;RandomAccessFile;;;Argument[0];create-file",

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

@@ -92,6 +92,37 @@ private class ApacheHttpXssSink extends SinkModelCsv {
   }
 }
 
+private class ApacheHttpOpenUrlSink extends SinkModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "org.apache.http;HttpRequest;true;setURI;;;Argument[0];open-url",
+        "org.apache.http.message;BasicHttpRequest;false;BasicHttpRequest;(RequestLine);;Argument[0];open-url",
+        "org.apache.http.message;BasicHttpRequest;false;BasicHttpRequest;(String,String);;Argument[1];open-url",
+        "org.apache.http.message;BasicHttpRequest;false;BasicHttpRequest;(String,String,ProtocolVersion);;Argument[1];open-url",
+        "org.apache.http.message;BasicHttpEntityEnclosingRequest;false;BasicHttpEntityEnclosingRequest;(RequestLine);;Argument[0];open-url",
+        "org.apache.http.message;BasicHttpEntityEnclosingRequest;false;BasicHttpEntityEnclosingRequest;(String,String);;Argument[1];open-url",
+        "org.apache.http.message;BasicHttpEntityEnclosingRequest;false;BasicHttpEntityEnclosingRequest;(String,String,ProtocolVersion);;Argument[1];open-url",
+        "org.apache.http.client.methods;HttpGet;false;HttpGet;;;Argument[0];open-url",
+        "org.apache.http.client.methods;HttpHead;false;HttpHead;;;Argument[0];open-url",
+        "org.apache.http.client.methods;HttpPut;false;HttpPut;;;Argument[0];open-url",
+        "org.apache.http.client.methods;HttpPost;false;HttpPost;;;Argument[0];open-url",
+        "org.apache.http.client.methods;HttpDelete;false;HttpDelete;;;Argument[0];open-url",
+        "org.apache.http.client.methods;HttpOptions;false;HttpOptions;;;Argument[0];open-url",
+        "org.apache.http.client.methods;HttpTrace;false;HttpTrace;;;Argument[0];open-url",
+        "org.apache.http.client.methods;HttpPatch;false;HttpPatch;;;Argument[0];open-url",
+        "org.apache.http.client.methods;HttpRequestBase;true;setURI;;;Argument[0];open-url",
+        "org.apache.http.client.methods;RequestBuilder;false;setUri;;;Argument[0];open-url",
+        "org.apache.http.client.methods;RequestBuilder;false;get;;;Argument[0];open-url",
+        "org.apache.http.client.methods;RequestBuilder;false;post;;;Argument[0];open-url",
+        "org.apache.http.client.methods;RequestBuilder;false;put;;;Argument[0];open-url",
+        "org.apache.http.client.methods;RequestBuilder;false;options;;;Argument[0];open-url",
+        "org.apache.http.client.methods;RequestBuilder;false;head;;;Argument[0];open-url",
+        "org.apache.http.client.methods;RequestBuilder;false;delete;;;Argument[0];open-url"
+      ]
+  }
+}
+
 private class ApacheHttpFlowStep extends SummaryModelCsv {
   override predicate row(string row) {
     row =

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

@@ -786,3 +786,9 @@ private class UriBuilderModel extends SummaryModelCsv {
       ]
   }
 }
+
+private class JaxRsUrlOpenSink extends SinkModelCsv {
+  override predicate row(string row) {
+    row = ["javax.ws.rs.client;Client;true;target;;;Argument[0];open-url"]
+  }
+}

java/ql/src/semmle/code/java/frameworks/spring/SpringHttp.qll

@@ -4,6 +4,7 @@
  */
 
 import java
+private import semmle.code.java.dataflow.ExternalFlow
 
 /** The class `org.springframework.http.HttpEntity` or an instantiation of it. */
 class SpringHttpEntity extends Class {
@@ -38,3 +39,25 @@ class SpringResponseEntityBodyBuilder extends Interface {
 class SpringHttpHeaders extends Class {
   SpringHttpHeaders() { this.hasQualifiedName("org.springframework.http", "HttpHeaders") }
 }
+
+private class UrlOpenSink extends SinkModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "org.springframework.http;RequestEntity;false;get;;;Argument[0];open-url",
+        "org.springframework.http;RequestEntity;false;post;;;Argument[0];open-url",
+        "org.springframework.http;RequestEntity;false;head;;;Argument[0];open-url",
+        "org.springframework.http;RequestEntity;false;delete;;;Argument[0];open-url",
+        "org.springframework.http;RequestEntity;false;options;;;Argument[0];open-url",
+        "org.springframework.http;RequestEntity;false;patch;;;Argument[0];open-url",
+        "org.springframework.http;RequestEntity;false;put;;;Argument[0];open-url",
+        "org.springframework.http;RequestEntity;false;method;;;Argument[1];open-url",
+        "org.springframework.http;RequestEntity;false;RequestEntity;(HttpMethod,URI);;Argument[1];open-url",
+        "org.springframework.http;RequestEntity;false;RequestEntity;(MultiValueMap,HttpMethod,URI);;Argument[2];open-url",
+        "org.springframework.http;RequestEntity;false;RequestEntity;(T,HttpMethod,URI);;Argument[2];open-url",
+        "org.springframework.http;RequestEntity;false;RequestEntity;(T,HttpMethod,URI,Type);;Argument[2];open-url",
+        "org.springframework.http;RequestEntity;false;RequestEntity;(T,MultiValueMap,HttpMethod,URI);;Argument[3];open-url",
+        "org.springframework.http;RequestEntity;false;RequestEntity;(T,MultiValueMap,HttpMethod,URI,Type);;Argument[3];open-url"
+      ]
+  }
+}

java/ql/src/semmle/code/java/frameworks/spring/SpringWebClient.qll

@@ -4,6 +4,7 @@
 
 import java
 import SpringHttp
+private import semmle.code.java.dataflow.ExternalFlow
 
 /** The class `org.springframework.web.client.RestTemplate`. */
 class SpringRestTemplate extends Class {
@@ -27,3 +28,21 @@ class SpringWebClient extends Interface {
     this.hasQualifiedName("org.springframework.web.reactive.function.client", "WebClient")
   }
 }
+
+private class UrlOpenSink extends SinkModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "org.springframework.web.client;RestTemplate;false;doExecute;;;Argument[0];open-url",
+        "org.springframework.web.client;RestTemplate;false;postForEntity;;;Argument[0];open-url",
+        "org.springframework.web.client;RestTemplate;false;postForLocation;;;Argument[0];open-url",
+        "org.springframework.web.client;RestTemplate;false;postForObject;;;Argument[0];open-url",
+        "org.springframework.web.client;RestTemplate;false;put;;;Argument[0];open-url",
+        "org.springframework.web.client;RestTemplate;false;exchange;;;Argument[0];open-url",
+        "org.springframework.web.client;RestTemplate;false;execute;;;Argument[0];open-url",
+        "org.springframework.web.client;RestTemplate;false;getForEntity;;;Argument[0];open-url",
+        "org.springframework.web.client;RestTemplate;false;getForObject;;;Argument[0];open-url",
+        "org.springframework.web.client;RestTemplate;false;patchForObject;;;Argument[0];open-url"
+      ]
+  }
+}

java/ql/src/semmle/code/java/security/RequestForgery.qll

@@ -9,6 +9,7 @@ import semmle.code.java.frameworks.javase.Http
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.dataflow.TaintTracking
 private import semmle.code.java.StringFormat
+private import semmle.code.java.dataflow.ExternalFlow
 
 /**
  * A unit class for adding additional taint steps that are specific to server-side request forgery (SSRF) attacks.
@@ -30,185 +31,14 @@ private class DefaultRequestForgeryAdditionalTaintStep extends RequestForgeryAdd
     or
     // propagate to a URL when its host is assigned to
     exists(UrlConstructorCall c | c.getHostArg() = pred.asExpr() | succ.asExpr() = c)
-    or
-    // propagate to a RequestEntity when its url is assigned to
-    exists(MethodAccess m |
-      m.getMethod().getDeclaringType() instanceof SpringRequestEntity and
-      (
-        m.getMethod().hasName(["get", "post", "head", "delete", "options", "patch", "put"]) and
-        m.getArgument(0) = pred.asExpr() and
-        m = succ.asExpr()
-        or
-        m.getMethod().hasName("method") and
-        m.getArgument(1) = pred.asExpr() and
-        m = succ.asExpr()
-      )
-    )
-    or
-    // propagate from a `RequestEntity<>$BodyBuilder` to a `RequestEntity`
-    // when the builder is tainted
-    exists(MethodAccess m, RefType t |
-      m.getMethod().getDeclaringType() = t and
-      t.hasQualifiedName("org.springframework.http", "RequestEntity<>$BodyBuilder") and
-      m.getMethod().hasName("body") and
-      m.getQualifier() = pred.asExpr() and
-      m = succ.asExpr()
-    )
   }
 }
 
 /** A data flow sink for server-side request forgery (SSRF) vulnerabilities. */
 abstract class RequestForgerySink extends DataFlow::Node { }
 
-/**
- * An argument to a url `openConnection` or `openStream` call
- * taken as a sink for request forgery vulnerabilities.
- */
-private class UrlOpen extends RequestForgerySink {
-  UrlOpen() {
-    exists(MethodAccess ma |
-      ma.getMethod() instanceof UrlOpenConnectionMethod or
-      ma.getMethod() instanceof UrlOpenStreamMethod
-    |
-      this.asExpr() = ma.getQualifier()
-    )
-  }
-}
-
-/**
- * An argument to an Apache `setURI` call taken as a
- * sink for request forgery vulnerabilities.
- */
-private class ApacheSetUri extends RequestForgerySink {
-  ApacheSetUri() {
-    exists(MethodAccess ma |
-      ma.getReceiverType() instanceof ApacheHttpRequest and
-      ma.getMethod().hasName("setURI")
-    |
-      this.asExpr() = ma.getArgument(0)
-    )
-  }
-}
-
-/**
- * An argument to any Apache `HttpRequest` instantiation taken as a
- * sink for request forgery vulnerabilities.
- */
-private class ApacheHttpRequestInstantiation extends RequestForgerySink {
-  ApacheHttpRequestInstantiation() {
-    exists(ClassInstanceExpr c | c.getConstructedType() instanceof ApacheHttpRequest |
-      this.asExpr() = c.getArgument(0)
-    )
-  }
-}
-
-/**
- * An argument to an Apache `RequestBuilder` method call taken as a
- * sink for request forgery vulnerabilities.
- */
-private class ApacheHttpRequestBuilderArgument extends RequestForgerySink {
-  ApacheHttpRequestBuilderArgument() {
-    exists(MethodAccess ma |
-      ma.getReceiverType() instanceof TypeApacheHttpRequestBuilder and
-      ma.getMethod().hasName(["setURI", "get", "post", "put", "optons", "head", "delete"])
-    |
-      this.asExpr() = ma.getArgument(0)
-    )
-  }
-}
-
-/**
- * An argument to any `java.net.http.HttpRequest` instantiation taken as a
- * sink for request forgery vulnerabilities.
- */
-private class HttpRequestNewBuilder extends RequestForgerySink {
-  HttpRequestNewBuilder() {
-    exists(MethodAccess call |
-      call.getCallee().hasName("newBuilder") and
-      call.getMethod().getDeclaringType().hasQualifiedName("java.net.http", "HttpRequest")
-    |
-      this.asExpr() = call.getArgument(0)
-    )
-  }
-}
-
-/**
- * An argument to an `HttpBuilder` `uri` call taken as a
- * sink for request forgery vulnerabilities.
- */
-private class HttpBuilderUriArgument extends RequestForgerySink {
-  HttpBuilderUriArgument() {
-    exists(MethodAccess ma | ma.getMethod() instanceof HttpBuilderUri |
-      this.asExpr() = ma.getArgument(0)
-    )
-  }
-}
-
-/**
- * An argument to a Spring `RestTemplate` method call taken as a
- * sink for request forgery vulnerabilities.
- */
-private class SpringRestTemplateArgument extends RequestForgerySink {
-  SpringRestTemplateArgument() {
-    this.asExpr() = any(SpringRestTemplateUrlMethodAccess m).getUrlArgument()
-  }
-}
-
-/**
- * An argument to a `javax.ws.rs.Client` `target` method call taken as a
- * sink for request forgery vulnerabilities.
- */
-private class JaxRsClientTarget extends RequestForgerySink {
-  JaxRsClientTarget() {
-    exists(MethodAccess ma |
-      ma.getMethod().getDeclaringType() instanceof JaxRsClient and
-      ma.getMethod().hasName("target")
-    |
-      this.asExpr() = ma.getArgument(0)
-    )
-  }
-}
-
-/**
- * A URI argument to an `org.springframework.http.RequestEntity` constructor call
- * taken as a sink for request forgery vulnerabilities.
- */
-private class RequestEntityUriArg extends RequestForgerySink {
-  RequestEntityUriArg() {
-    exists(ClassInstanceExpr e, Argument a |
-      e.getConstructedType() instanceof SpringRequestEntity and
-      e.getAnArgument() = a and
-      a.getType() instanceof TypeUri and
-      this.asExpr() = a
-    )
-  }
-}
-
-/**
- * A Spring Rest Template method
- * that takes a URL as an argument.
- */
-private class SpringRestTemplateUrlMethod extends Method {
-  SpringRestTemplateUrlMethod() {
-    this.getDeclaringType() instanceof SpringRestTemplate and
-    this.hasName([
-        "doExecute", "postForEntity", "postForLocation", "postForObject", "put", "exchange",
-        "execute", "getForEntity", "getForObject", "patchForObject"
-      ])
-  }
-}
-
-/**
- * A call to a Spring Rest Template method
- * that takes a URL as an argument.
- */
-private class SpringRestTemplateUrlMethodAccess extends MethodAccess {
-  SpringRestTemplateUrlMethodAccess() { this.getMethod() instanceof SpringRestTemplateUrlMethod }
-
-  /**
-   * Gets the URL argument of this template call.
-   */
-  Argument getUrlArgument() { result = this.getArgument(0) }
+private class UrlOpenSinkAsRequestForgerySink extends RequestForgerySink {
+  UrlOpenSinkAsRequestForgerySink() { sinkNode(this, "open-url") }
 }
 
 /** A sanitizer for request forgery vulnerabilities. */