Skip to content

Commit d48adbd

Browse files
committed
Refactor JsonpInjection
1 parent 8cb5e78 commit d48adbd

File tree

3 files changed

+36
-37
lines changed

3 files changed

+36
-37
lines changed

java/ql/src/experimental/Security/CWE/CWE-352/JsonStringLib.qll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import java
22
import semmle.code.java.dataflow.DataFlow
33
import semmle.code.java.dataflow.FlowSources
4-
import DataFlow::PathGraph
54

65
/** Json string type data. */
76
abstract class JsonStringSource extends DataFlow::Node { }

java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjection.ql

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,35 +12,37 @@
1212
*/
1313

1414
import java
15-
import JsonpInjectionLib
15+
import semmle.code.java.dataflow.TaintTracking
1616
import semmle.code.java.dataflow.FlowSources
1717
import semmle.code.java.deadcode.WebEntryPoints
18-
import DataFlow::PathGraph
18+
import semmle.code.java.security.XSS
19+
import JsonpInjectionLib
20+
import RequestResponseFlow::PathGraph
1921

2022
/** Taint-tracking configuration tracing flow from get method request sources to output jsonp data. */
21-
class RequestResponseFlowConfig extends TaintTracking::Configuration {
22-
RequestResponseFlowConfig() { this = "RequestResponseFlowConfig" }
23-
24-
override predicate isSource(DataFlow::Node source) {
23+
module RequestResponseFlowConfig implements DataFlow::ConfigSig {
24+
predicate isSource(DataFlow::Node source) {
2525
source instanceof RemoteFlowSource and
2626
any(RequestGetMethod m).polyCalls*(source.getEnclosingCallable())
2727
}
2828

29-
override predicate isSink(DataFlow::Node sink) {
29+
predicate isSink(DataFlow::Node sink) {
3030
sink instanceof XssSink and
3131
any(RequestGetMethod m).polyCalls*(sink.getEnclosingCallable())
3232
}
3333

34-
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
34+
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
3535
exists(MethodAccess ma |
3636
isRequestGetParamMethod(ma) and pred.asExpr() = ma.getQualifier() and succ.asExpr() = ma
3737
)
3838
}
3939
}
4040

41-
from DataFlow::PathNode source, DataFlow::PathNode sink, RequestResponseFlowConfig conf
41+
module RequestResponseFlow = TaintTracking::Global<RequestResponseFlowConfig>;
42+
43+
from RequestResponseFlow::PathNode source, RequestResponseFlow::PathNode sink
4244
where
43-
conf.hasFlowPath(source, sink) and
44-
exists(JsonpInjectionFlowConfig jhfc | jhfc.hasFlowTo(sink.getNode()))
45+
RequestResponseFlow::flowPath(source, sink) and
46+
JsonpInjectionFlow::flowTo(sink.getNode())
4547
select sink.getNode(), source, sink, "Jsonp response might include code from $@.", source.getNode(),
4648
"this user input"

java/ql/src/experimental/Security/CWE/CWE-352/JsonpInjectionLib.qll

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
import java
2-
import DataFlow
3-
import JsonStringLib
4-
import semmle.code.java.security.XSS
5-
import semmle.code.java.dataflow.DataFlow
6-
import semmle.code.java.dataflow.DataFlow3
7-
import semmle.code.java.dataflow.FlowSources
8-
import semmle.code.java.frameworks.spring.SpringController
2+
private import JsonStringLib
3+
private import semmle.code.java.security.XSS
4+
private import semmle.code.java.dataflow.TaintTracking
5+
private import semmle.code.java.dataflow.FlowSources
6+
private import semmle.code.java.frameworks.spring.SpringController
97

108
/**
119
* A method that is called to handle an HTTP GET request.
@@ -81,38 +79,38 @@ class JsonpBuilderExpr extends AddExpr {
8179
}
8280

8381
/** A data flow configuration tracing flow from remote sources to jsonp function name. */
84-
class RemoteFlowConfig extends DataFlow2::Configuration {
85-
RemoteFlowConfig() { this = "RemoteFlowConfig" }
82+
module RemoteFlowConfig implements DataFlow::ConfigSig {
83+
predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
8684

87-
override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
88-
89-
override predicate isSink(DataFlow::Node sink) {
85+
predicate isSink(DataFlow::Node sink) {
9086
exists(JsonpBuilderExpr jhe | jhe.getFunctionName() = sink.asExpr())
9187
}
9288
}
9389

94-
/** A data flow configuration tracing flow from json data into the argument `json` of JSONP-like string `someFunctionName + "(" + json + ")"`. */
95-
class JsonDataFlowConfig extends DataFlow2::Configuration {
96-
JsonDataFlowConfig() { this = "JsonDataFlowConfig" }
90+
module RemoteFlow = DataFlow::Global<RemoteFlowConfig>;
9791

98-
override predicate isSource(DataFlow::Node src) { src instanceof JsonStringSource }
92+
/** A data flow configuration tracing flow from json data into the argument `json` of JSONP-like string `someFunctionName + "(" + json + ")"`. */
93+
module JsonDataFlowConfig implements DataFlow::ConfigSig {
94+
predicate isSource(DataFlow::Node src) { src instanceof JsonStringSource }
9995

100-
override predicate isSink(DataFlow::Node sink) {
96+
predicate isSink(DataFlow::Node sink) {
10197
exists(JsonpBuilderExpr jhe | jhe.getJsonExpr() = sink.asExpr())
10298
}
10399
}
104100

105-
/** Taint-tracking configuration tracing flow from probable jsonp data with a user-controlled function name to an outgoing HTTP entity. */
106-
class JsonpInjectionFlowConfig extends TaintTracking::Configuration {
107-
JsonpInjectionFlowConfig() { this = "JsonpInjectionFlowConfig" }
101+
module JsonDataFlow = DataFlow::Global<JsonDataFlowConfig>;
108102

109-
override predicate isSource(DataFlow::Node src) {
110-
exists(JsonpBuilderExpr jhe, JsonDataFlowConfig jdfc, RemoteFlowConfig rfc |
103+
/** Taint-tracking configuration tracing flow from probable jsonp data with a user-controlled function name to an outgoing HTTP entity. */
104+
module JsonpInjectionFlowConfig implements DataFlow::ConfigSig {
105+
predicate isSource(DataFlow::Node src) {
106+
exists(JsonpBuilderExpr jhe |
111107
jhe = src.asExpr() and
112-
jdfc.hasFlowTo(DataFlow::exprNode(jhe.getJsonExpr())) and
113-
rfc.hasFlowTo(DataFlow::exprNode(jhe.getFunctionName()))
108+
JsonDataFlow::flowTo(DataFlow::exprNode(jhe.getJsonExpr())) and
109+
RemoteFlow::flowTo(DataFlow::exprNode(jhe.getFunctionName()))
114110
)
115111
}
116112

117-
override predicate isSink(DataFlow::Node sink) { sink instanceof XssSink }
113+
predicate isSink(DataFlow::Node sink) { sink instanceof XssSink }
118114
}
115+
116+
module JsonpInjectionFlow = TaintTracking::Global<JsonpInjectionFlowConfig>;

0 commit comments

Comments
 (0)