Skip to content

Commit c44254e

Browse files
committed
Refactor XsltInjection
1 parent 559f6a5 commit c44254e

File tree

3 files changed

+38
-16
lines changed

3 files changed

+38
-16
lines changed

java/ql/lib/semmle/code/java/security/XsltInjectionQuery.qll

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@ import semmle.code.java.security.XmlParsers
77
import semmle.code.java.security.XsltInjection
88

99
/**
10+
* DEPRECATED: Use `XsltInjectionFlow` instead.
11+
*
1012
* A taint-tracking configuration for unvalidated user input that is used in XSLT transformation.
1113
*/
12-
class XsltInjectionFlowConfig extends TaintTracking::Configuration {
14+
deprecated class XsltInjectionFlowConfig extends TaintTracking::Configuration {
1315
XsltInjectionFlowConfig() { this = "XsltInjectionFlowConfig" }
1416

1517
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -25,6 +27,28 @@ class XsltInjectionFlowConfig extends TaintTracking::Configuration {
2527
}
2628
}
2729

30+
/**
31+
* A taint-tracking configuration for unvalidated user input that is used in XSLT transformation.
32+
*/
33+
private module XsltInjectionFlowConfig implements DataFlow::ConfigSig {
34+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
35+
36+
predicate isSink(DataFlow::Node sink) { sink instanceof XsltInjectionSink }
37+
38+
predicate isBarrier(DataFlow::Node node) {
39+
node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
40+
}
41+
42+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
43+
any(XsltInjectionAdditionalTaintStep c).step(node1, node2)
44+
}
45+
}
46+
47+
/**
48+
* Tracks flow from unvalidated user input to XSLT transformation.
49+
*/
50+
module XsltInjectionFlow = TaintTracking::Make<XsltInjectionFlowConfig>;
51+
2852
/**
2953
* A set of additional taint steps to consider when taint tracking XSLT related data flows.
3054
* These steps use data flow logic themselves.
@@ -46,39 +70,37 @@ private predicate newTransformerOrTemplatesStep(DataFlow::Node n1, DataFlow::Nod
4670
n2.asExpr() = ma and
4771
m.getDeclaringType() instanceof TransformerFactory and
4872
m.hasName(["newTransformer", "newTemplates"]) and
49-
not exists(TransformerFactoryWithSecureProcessingFeatureFlowConfig conf |
50-
conf.hasFlowToExpr(ma.getQualifier())
51-
)
73+
not TransformerFactoryWithSecureProcessingFeatureFlow::hasFlowToExpr(ma.getQualifier())
5274
)
5375
}
5476

5577
/**
5678
* A data flow configuration for secure processing feature that is enabled on `TransformerFactory`.
5779
*/
58-
private class TransformerFactoryWithSecureProcessingFeatureFlowConfig extends DataFlow2::Configuration
80+
private module TransformerFactoryWithSecureProcessingFeatureFlowConfig implements
81+
DataFlow::ConfigSig
5982
{
60-
TransformerFactoryWithSecureProcessingFeatureFlowConfig() {
61-
this = "TransformerFactoryWithSecureProcessingFeatureFlowConfig"
62-
}
63-
64-
override predicate isSource(DataFlow::Node src) {
83+
predicate isSource(DataFlow::Node src) {
6584
exists(Variable v | v = src.asExpr().(VarAccess).getVariable() |
6685
exists(TransformerFactoryFeatureConfig config | config.getQualifier() = v.getAnAccess() |
6786
config.enables(configSecureProcessing())
6887
)
6988
)
7089
}
7190

72-
override predicate isSink(DataFlow::Node sink) {
91+
predicate isSink(DataFlow::Node sink) {
7392
exists(MethodAccess ma |
7493
sink.asExpr() = ma.getQualifier() and
7594
ma.getMethod().getDeclaringType() instanceof TransformerFactory
7695
)
7796
}
7897

79-
override int fieldFlowBranchLimit() { result = 0 }
98+
int fieldFlowBranchLimit() { result = 0 }
8099
}
81100

101+
private module TransformerFactoryWithSecureProcessingFeatureFlow =
102+
DataFlow::Make<TransformerFactoryWithSecureProcessingFeatureFlowConfig>;
103+
82104
/** A `ParserConfig` specific to `TransformerFactory`. */
83105
private class TransformerFactoryFeatureConfig extends ParserConfig {
84106
TransformerFactoryFeatureConfig() {

java/ql/src/Security/CWE/CWE-074/XsltInjection.ql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313

1414
import java
1515
import semmle.code.java.security.XsltInjectionQuery
16-
import DataFlow::PathGraph
16+
import XsltInjectionFlow::PathGraph
1717

18-
from DataFlow::PathNode source, DataFlow::PathNode sink, XsltInjectionFlowConfig conf
19-
where conf.hasFlowPath(source, sink)
18+
from XsltInjectionFlow::PathNode source, XsltInjectionFlow::PathNode sink
19+
where XsltInjectionFlow::hasFlowPath(source, sink)
2020
select sink.getNode(), source, sink, "XSLT transformation might include stylesheet from $@.",
2121
source.getNode(), "this user input"

java/ql/test/query-tests/security/CWE-074/XsltInjectionTest.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ class HasXsltInjectionTest extends InlineExpectationsTest {
1111

1212
override predicate hasActualResult(Location location, string element, string tag, string value) {
1313
tag = "hasXsltInjection" and
14-
exists(DataFlow::Node sink, XsltInjectionFlowConfig conf | conf.hasFlowTo(sink) |
14+
exists(DataFlow::Node sink | XsltInjectionFlow::hasFlowTo(sink) |
1515
sink.getLocation() = location and
1616
element = sink.toString() and
1717
value = ""

0 commit comments

Comments
 (0)