diff --git a/java/ql/src/experimental/Security/CWE/CWE-352/JsonStringLib.qll b/java/ql/src/experimental/Security/CWE/CWE-352/JsonStringLib.qll index f54d7cfaa03..2edf03d02ef 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-352/JsonStringLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-352/JsonStringLib.qll @@ -1,7 +1,6 @@ import java import semmle.code.java.dataflow.DataFlow import semmle.code.java.dataflow.FlowSources -import DataFlow::PathGraph /** Json string type data. */ abstract class JsonStringSource extends DataFlow::Node { } diff --git a/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.ql index 9518a04ff6f..847f6eecdef 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.ql @@ -12,35 +12,37 @@ */ import java -import JsonpInjectionLib +import semmle.code.java.dataflow.TaintTracking import semmle.code.java.dataflow.FlowSources import semmle.code.java.deadcode.WebEntryPoints -import DataFlow::PathGraph +import semmle.code.java.security.XSS +import JsonpInjectionLib +import RequestResponseFlow::PathGraph /** Taint-tracking configuration tracing flow from get method request sources to output jsonp data. */ -class RequestResponseFlowConfig extends TaintTracking::Configuration { - RequestResponseFlowConfig() { this = "RequestResponseFlowConfig" } - - override predicate isSource(DataFlow::Node source) { +module RequestResponseFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource and any(RequestGetMethod m).polyCalls*(source.getEnclosingCallable()) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink instanceof XssSink and any(RequestGetMethod m).polyCalls*(sink.getEnclosingCallable()) } - override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { exists(MethodAccess ma | isRequestGetParamMethod(ma) and pred.asExpr() = ma.getQualifier() and succ.asExpr() = ma ) } } -from DataFlow::PathNode source, DataFlow::PathNode sink, RequestResponseFlowConfig conf +module RequestResponseFlow = TaintTracking::Global; + +from RequestResponseFlow::PathNode source, RequestResponseFlow::PathNode sink where - conf.hasFlowPath(source, sink) and - exists(JsonpInjectionFlowConfig jhfc | jhfc.hasFlowTo(sink.getNode())) + RequestResponseFlow::flowPath(source, sink) and + JsonpInjectionFlow::flowTo(sink.getNode()) select sink.getNode(), source, sink, "Jsonp response might include code from $@.", source.getNode(), "this user input" diff --git a/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjectionLib.qll index cc1552b59b9..7076ffddbce 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjectionLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjectionLib.qll @@ -1,11 +1,9 @@ import java -import DataFlow -import JsonStringLib -import semmle.code.java.security.XSS -import semmle.code.java.dataflow.DataFlow -import semmle.code.java.dataflow.DataFlow3 -import semmle.code.java.dataflow.FlowSources -import semmle.code.java.frameworks.spring.SpringController +private import JsonStringLib +private import semmle.code.java.security.XSS +private import semmle.code.java.dataflow.TaintTracking +private import semmle.code.java.dataflow.FlowSources +private import semmle.code.java.frameworks.spring.SpringController /** * A method that is called to handle an HTTP GET request. @@ -81,38 +79,38 @@ class JsonpBuilderExpr extends AddExpr { } /** A data flow configuration tracing flow from remote sources to jsonp function name. */ -class RemoteFlowConfig extends DataFlow2::Configuration { - RemoteFlowConfig() { this = "RemoteFlowConfig" } +module RemoteFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } - override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(JsonpBuilderExpr jhe | jhe.getFunctionName() = sink.asExpr()) } } +module RemoteFlow = DataFlow::Global; + /** A data flow configuration tracing flow from json data into the argument `json` of JSONP-like string `someFunctionName + "(" + json + ")"`. */ -class JsonDataFlowConfig extends DataFlow2::Configuration { - JsonDataFlowConfig() { this = "JsonDataFlowConfig" } +module JsonDataFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src instanceof JsonStringSource } - override predicate isSource(DataFlow::Node src) { src instanceof JsonStringSource } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(JsonpBuilderExpr jhe | jhe.getJsonExpr() = sink.asExpr()) } } -/** Taint-tracking configuration tracing flow from probable jsonp data with a user-controlled function name to an outgoing HTTP entity. */ -class JsonpInjectionFlowConfig extends TaintTracking::Configuration { - JsonpInjectionFlowConfig() { this = "JsonpInjectionFlowConfig" } +module JsonDataFlow = DataFlow::Global; - override predicate isSource(DataFlow::Node src) { - exists(JsonpBuilderExpr jhe, JsonDataFlowConfig jdfc, RemoteFlowConfig rfc | +/** Taint-tracking configuration tracing flow from probable jsonp data with a user-controlled function name to an outgoing HTTP entity. */ +module JsonpInjectionFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { + exists(JsonpBuilderExpr jhe | jhe = src.asExpr() and - jdfc.hasFlowTo(DataFlow::exprNode(jhe.getJsonExpr())) and - rfc.hasFlowTo(DataFlow::exprNode(jhe.getFunctionName())) + JsonDataFlow::flowTo(DataFlow::exprNode(jhe.getJsonExpr())) and + RemoteFlow::flowTo(DataFlow::exprNode(jhe.getFunctionName())) ) } - override predicate isSink(DataFlow::Node sink) { sink instanceof XssSink } + predicate isSink(DataFlow::Node sink) { sink instanceof XssSink } } + +module JsonpInjectionFlow = TaintTracking::Global;