diff --git a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll index 71b11b0900b..d23cfe5b5d4 100644 --- a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll +++ b/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", diff --git a/java/ql/src/semmle/code/java/frameworks/ApacheHttp.qll b/java/ql/src/semmle/code/java/frameworks/ApacheHttp.qll index 80cb589e6f2..e006c245ecb 100644 --- a/java/ql/src/semmle/code/java/frameworks/ApacheHttp.qll +++ b/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 = diff --git a/java/ql/src/semmle/code/java/frameworks/JaxWS.qll b/java/ql/src/semmle/code/java/frameworks/JaxWS.qll index bfe332da2b6..0ca3175b58d 100644 --- a/java/ql/src/semmle/code/java/frameworks/JaxWS.qll +++ b/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"] + } +} diff --git a/java/ql/src/semmle/code/java/frameworks/spring/SpringHttp.qll b/java/ql/src/semmle/code/java/frameworks/spring/SpringHttp.qll index 59016df25f8..c56329d3a5a 100644 --- a/java/ql/src/semmle/code/java/frameworks/spring/SpringHttp.qll +++ b/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" + ] + } +} diff --git a/java/ql/src/semmle/code/java/frameworks/spring/SpringWebClient.qll b/java/ql/src/semmle/code/java/frameworks/spring/SpringWebClient.qll index 3a8d4bb084a..b76723a3bcc 100644 --- a/java/ql/src/semmle/code/java/frameworks/spring/SpringWebClient.qll +++ b/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" + ] + } +} diff --git a/java/ql/src/semmle/code/java/security/RequestForgery.qll b/java/ql/src/semmle/code/java/security/RequestForgery.qll index 8a0087bb9fb..7ee59fec8dd 100644 --- a/java/ql/src/semmle/code/java/security/RequestForgery.qll +++ b/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. */ diff --git a/java/ql/test/query-tests/security/CWE-918/RequestForgery.expected b/java/ql/test/query-tests/security/CWE-918/RequestForgery.expected index 8c50a69c1b7..ad5b0625a77 100644 --- a/java/ql/test/query-tests/security/CWE-918/RequestForgery.expected +++ b/java/ql/test/query-tests/security/CWE-918/RequestForgery.expected @@ -48,12 +48,15 @@ edges | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:37:43:37:56 | fooResourceUrl | | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:41:42:41:55 | fooResourceUrl | | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:45:47:45:60 | fooResourceUrl | +| SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:50:40:50:62 | new URI(...) | +| SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:50:48:50:61 | fooResourceUrl : String | | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:54:59:54:72 | fooResourceUrl | | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:58:74:58:96 | new URI(...) | | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:58:82:58:95 | fooResourceUrl : String | | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:62:57:62:70 | fooResourceUrl | | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:66:48:66:61 | fooResourceUrl | | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:69:30:69:43 | fooResourceUrl | +| SpringSSRF.java:50:48:50:61 | fooResourceUrl : String | SpringSSRF.java:50:40:50:62 | new URI(...) | | SpringSSRF.java:58:82:58:95 | fooResourceUrl : String | SpringSSRF.java:58:74:58:96 | new URI(...) | nodes | JaxWsSSRF.java:21:22:21:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | @@ -106,6 +109,8 @@ nodes | SpringSSRF.java:37:43:37:56 | fooResourceUrl | semmle.label | fooResourceUrl | | SpringSSRF.java:41:42:41:55 | fooResourceUrl | semmle.label | fooResourceUrl | | SpringSSRF.java:45:47:45:60 | fooResourceUrl | semmle.label | fooResourceUrl | +| SpringSSRF.java:50:40:50:62 | new URI(...) | semmle.label | new URI(...) | +| SpringSSRF.java:50:48:50:61 | fooResourceUrl : String | semmle.label | fooResourceUrl : String | | SpringSSRF.java:54:59:54:72 | fooResourceUrl | semmle.label | fooResourceUrl | | SpringSSRF.java:58:74:58:96 | new URI(...) | semmle.label | new URI(...) | | SpringSSRF.java:58:82:58:95 | fooResourceUrl : String | semmle.label | fooResourceUrl : String | @@ -136,6 +141,7 @@ nodes | SpringSSRF.java:37:43:37:56 | fooResourceUrl | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:37:43:37:56 | fooResourceUrl | Potential server-side request forgery due to $@. | SpringSSRF.java:26:33:26:60 | getParameter(...) | a user-provided value | | SpringSSRF.java:41:42:41:55 | fooResourceUrl | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:41:42:41:55 | fooResourceUrl | Potential server-side request forgery due to $@. | SpringSSRF.java:26:33:26:60 | getParameter(...) | a user-provided value | | SpringSSRF.java:45:47:45:60 | fooResourceUrl | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:45:47:45:60 | fooResourceUrl | Potential server-side request forgery due to $@. | SpringSSRF.java:26:33:26:60 | getParameter(...) | a user-provided value | +| SpringSSRF.java:50:40:50:62 | new URI(...) | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:50:40:50:62 | new URI(...) | Potential server-side request forgery due to $@. | SpringSSRF.java:26:33:26:60 | getParameter(...) | a user-provided value | | SpringSSRF.java:54:59:54:72 | fooResourceUrl | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:54:59:54:72 | fooResourceUrl | Potential server-side request forgery due to $@. | SpringSSRF.java:26:33:26:60 | getParameter(...) | a user-provided value | | SpringSSRF.java:58:74:58:96 | new URI(...) | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:58:74:58:96 | new URI(...) | Potential server-side request forgery due to $@. | SpringSSRF.java:26:33:26:60 | getParameter(...) | a user-provided value | | SpringSSRF.java:62:57:62:70 | fooResourceUrl | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:62:57:62:70 | fooResourceUrl | Potential server-side request forgery due to $@. | SpringSSRF.java:26:33:26:60 | getParameter(...) | a user-provided value |