diff --git a/java/ql/lib/semmle/code/java/frameworks/Networking.qll b/java/ql/lib/semmle/code/java/frameworks/Networking.qll index cad948ed2f4..10a1999b8b8 100644 --- a/java/ql/lib/semmle/code/java/frameworks/Networking.qll +++ b/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,6 +83,8 @@ 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`. */ @@ -81,6 +92,8 @@ 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 diff --git a/java/ql/lib/semmle/code/java/security/HttpsUrls.qll b/java/ql/lib/semmle/code/java/security/HttpsUrls.qll new file mode 100644 index 00000000000..f3ce2ac11a5 --- /dev/null +++ b/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() + ) +} diff --git a/java/ql/lib/semmle/code/java/security/HttpsUrlsQuery.qll b/java/ql/lib/semmle/code/java/security/HttpsUrlsQuery.qll new file mode 100644 index 00000000000..50c76cefbca --- /dev/null +++ b/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 + } +} diff --git a/java/ql/src/Security/CWE/CWE-319/HttpsUrls.ql b/java/ql/src/Security/CWE/CWE-319/HttpsUrls.ql index 001afb8efb1..da5d847b4dd 100644 --- a/java/ql/src/Security/CWE/CWE-319/HttpsUrls.ql +++ b/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