Skip to content

Commit 8e54328

Browse files
egregius313egregius313
authored
Merge pull request github#12681 from egregius313/egregius313/java/move-configurations-to-libraries
Java: Move dataflow configurations in queries to `*Query.qll` libraries (part 1)
2 parents 3cd0af6 + 684408a commit 8e54328

17 files changed

+331
-282
lines changed

‎java/ql/lib/change-notes/2023-03-29-moved-configurations-for-queries.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added the `TaintedPathQuery.qll` library to provide the `TaintedPathFlow` and `TaintedPathLocalFlow` taint-tracking modules to reason about tainted path vulnerabilities.
5+
* Added the `ZipSlipQuery.qll` library to provide the `ZipSlipFlow` taint-tracking module to reason about zip-slip vulnerabilities.
6+
* Added the `InsecureBeanValidationQuery.qll` library to provide the `BeanValidationFlow` taint-tracking module to reason about bean validation vulnerabilities.
7+
* Added the `XssQuery.qll` library to provide the `XssFlow` taint-tracking module to reason about cross site scripting vulnerabilities.
8+
* Added the `LdapInjectionQuery.qll` library to provide the `LdapInjectionFlow` taint-tracking module to reason about LDAP injection vulnerabilities.
9+
* Added the `ResponseSplittingQuery.qll` library to provide the `ResponseSplittingFlow` taint-tracking module to reason about response splitting vulnerabilities.
10+
* Added the `ExternallyControlledFormatStringQuery.qll` library to provide the `ExternallyControlledFormatStringFlow` taint-tracking module to reason about externally controlled format string vulnerabilities.

‎java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringQuery.qll

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/** Provides a taint-tracking configuration to reason about externally controlled format string vulnerabilities. */
2+
3+
import java
4+
private import semmle.code.java.dataflow.FlowSources
5+
private import semmle.code.java.StringFormat
6+
7+
/**
8+
* A taint-tracking configuration for externally controlled format string vulnerabilities.
9+
*/
10+
module ExternallyControlledFormatStringConfig implements DataFlow::ConfigSig {
11+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
12+
13+
predicate isSink(DataFlow::Node sink) {
14+
sink.asExpr() = any(StringFormat formatCall).getFormatArgument()
15+
}
16+
17+
predicate isBarrier(DataFlow::Node node) {
18+
node.getType() instanceof NumericType or node.getType() instanceof BooleanType
19+
}
20+
}
21+
22+
/**
23+
* Taint-tracking flow for externally controlled format string vulnerabilities.
24+
*/
25+
module ExternallyControlledFormatStringFlow =
26+
TaintTracking::Global<ExternallyControlledFormatStringConfig>;

‎java/ql/lib/semmle/code/java/security/InsecureBeanValidationQuery.qll

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/** Provides classes and a taint tracking configuration to reason about insecure bean validation. */
2+
3+
import java
4+
import semmle.code.java.dataflow.TaintTracking
5+
import semmle.code.java.dataflow.FlowSources
6+
private import semmle.code.java.dataflow.ExternalFlow
7+
8+
/**
9+
* A message interpolator Type that perform Expression Language (EL) evaluations.
10+
*/
11+
private class ELMessageInterpolatorType extends RefType {
12+
ELMessageInterpolatorType() {
13+
this.getASourceSupertype*()
14+
.hasQualifiedName("org.hibernate.validator.messageinterpolation",
15+
["ResourceBundleMessageInterpolator", "ValueFormatterMessageInterpolator"])
16+
}
17+
}
18+
19+
/**
20+
* A method call that sets the application's default message interpolator.
21+
*/
22+
class SetMessageInterpolatorCall extends MethodAccess {
23+
SetMessageInterpolatorCall() {
24+
exists(Method m, RefType t |
25+
this.getMethod() = m and
26+
m.getDeclaringType().getASourceSupertype*() = t and
27+
(
28+
t.hasQualifiedName("javax.validation", ["Configuration", "ValidatorContext"]) and
29+
m.getName() = "messageInterpolator"
30+
or
31+
t.hasQualifiedName("org.springframework.validation.beanvalidation",
32+
["CustomValidatorBean", "LocalValidatorFactoryBean"]) and
33+
m.getName() = "setMessageInterpolator"
34+
)
35+
)
36+
}
37+
38+
/**
39+
* Holds if the message interpolator is likely to be safe, because it does not process Java Expression Language expressions.
40+
*/
41+
predicate isSafe() { not this.getAnArgument().getType() instanceof ELMessageInterpolatorType }
42+
}
43+
44+
/**
45+
* Taint tracking BeanValidationConfiguration describing the flow of data from user input
46+
* to the argument of a method that builds constraint error messages.
47+
*/
48+
module BeanValidationConfig implements DataFlow::ConfigSig {
49+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
50+
51+
predicate isSink(DataFlow::Node sink) { sink instanceof BeanValidationSink }
52+
}
53+
54+
/** Tracks flow from user input to the argument of a method that builds constraint error messages. */
55+
module BeanValidationFlow = TaintTracking::Global<BeanValidationConfig>;
56+
57+
/**
58+
* A bean validation sink, such as method `buildConstraintViolationWithTemplate`
59+
* declared on a subtype of `javax.validation.ConstraintValidatorContext`.
60+
*/
61+
private class BeanValidationSink extends DataFlow::Node {
62+
BeanValidationSink() { sinkNode(this, "bean-validation") }
63+
}

‎java/ql/src/Security/CWE/CWE-090/LdapInjectionLib.qll renamed to ‎java/ql/lib/semmle/code/java/security/LdapInjectionQuery.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
/** Provides a taint tracking configuration to reason about unvalidated user input that is used to construct LDAP queries. */
2+
13
import java
24
import semmle.code.java.dataflow.FlowSources
35
import semmle.code.java.security.LdapInjection
@@ -17,4 +19,5 @@ module LdapInjectionFlowConfig implements DataFlow::ConfigSig {
1719
}
1820
}
1921

22+
/** Tracks flow from remote sources to LDAP injection vulnerabilities. */
2023
module LdapInjectionFlow = TaintTracking::Global<LdapInjectionFlowConfig>;

‎java/ql/lib/semmle/code/java/security/ResponseSplittingQuery.qll

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/** Provides a taint tracking configuration to reason about response splitting vulnerabilities. */
2+
3+
import java
4+
private import semmle.code.java.dataflow.FlowSources
5+
import semmle.code.java.security.ResponseSplitting
6+
7+
/**
8+
* A taint-tracking configuration for response splitting vulnerabilities.
9+
*/
10+
module ResponseSplittingConfig implements DataFlow::ConfigSig {
11+
predicate isSource(DataFlow::Node source) {
12+
source instanceof RemoteFlowSource and
13+
not source instanceof SafeHeaderSplittingSource
14+
}
15+
16+
predicate isSink(DataFlow::Node sink) { sink instanceof HeaderSplittingSink }
17+
18+
predicate isBarrier(DataFlow::Node node) {
19+
node.getType() instanceof PrimitiveType
20+
or
21+
node.getType() instanceof BoxedType
22+
or
23+
exists(MethodAccess ma, string methodName, CompileTimeConstantExpr target |
24+
node.asExpr() = ma and
25+
ma.getMethod().hasQualifiedName("java.lang", "String", methodName) and
26+
target = ma.getArgument(0) and
27+
(
28+
methodName = "replace" and target.getIntValue() = [10, 13] // 10 == "\n", 13 == "\r"
29+
or
30+
methodName = "replaceAll" and
31+
target.getStringValue().regexpMatch(".*([\n\r]|\\[\\^[^\\]\r\n]*\\]).*")
32+
)
33+
)
34+
}
35+
}
36+
37+
/**
38+
* Tracks flow from remote sources to response splitting vulnerabilities.
39+
*/
40+
module ResponseSplittingFlow = TaintTracking::Global<ResponseSplittingConfig>;

‎java/ql/lib/semmle/code/java/security/TaintedPathQuery.qll

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/** Provides dataflow configurations for tainted path queries. */
2+
3+
import java
4+
import semmle.code.java.frameworks.Networking
5+
import semmle.code.java.dataflow.DataFlow
6+
import semmle.code.java.dataflow.FlowSources
7+
private import semmle.code.java.dataflow.ExternalFlow
8+
import semmle.code.java.security.PathCreation
9+
import semmle.code.java.security.PathSanitizer
10+
11+
/**
12+
* A unit class for adding additional taint steps.
13+
*
14+
* Extend this class to add additional taint steps that should apply to tainted path flow configurations.
15+
*/
16+
class TaintedPathAdditionalTaintStep extends Unit {
17+
abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
18+
}
19+
20+
private class DefaultTaintedPathAdditionalTaintStep extends TaintedPathAdditionalTaintStep {
21+
override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
22+
exists(Argument a |
23+
a = n1.asExpr() and
24+
a.getCall() = n2.asExpr() and
25+
a = any(TaintPreservingUriCtorParam tpp).getAnArgument()
26+
)
27+
}
28+
}
29+
30+
private class TaintPreservingUriCtorParam extends Parameter {
31+
TaintPreservingUriCtorParam() {
32+
exists(Constructor ctor, int idx, int nParams |
33+
ctor.getDeclaringType() instanceof TypeUri and
34+
this = ctor.getParameter(idx) and
35+
nParams = ctor.getNumberOfParameters()
36+
|
37+
// URI(String scheme, String ssp, String fragment)
38+
idx = 1 and nParams = 3
39+
or
40+
// URI(String scheme, String host, String path, String fragment)
41+
idx = [1, 2] and nParams = 4
42+
or
43+
// URI(String scheme, String authority, String path, String query, String fragment)
44+
idx = 2 and nParams = 5
45+
or
46+
// URI(String scheme, String userInfo, String host, int port, String path, String query, String fragment)
47+
idx = 4 and nParams = 7
48+
)
49+
}
50+
}
51+
52+
/**
53+
* A taint-tracking configuration for tracking flow from remote sources to the creation of a path.
54+
*/
55+
module TaintedPathConfig implements DataFlow::ConfigSig {
56+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
57+
58+
predicate isSink(DataFlow::Node sink) {
59+
sink.asExpr() = any(PathCreation p).getAnInput()
60+
or
61+
sinkNode(sink, ["create-file", "read-file"])
62+
}
63+
64+
predicate isBarrier(DataFlow::Node sanitizer) {
65+
sanitizer.getType() instanceof BoxedType or
66+
sanitizer.getType() instanceof PrimitiveType or
67+
sanitizer.getType() instanceof NumberType or
68+
sanitizer instanceof PathInjectionSanitizer
69+
}
70+
71+
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
72+
any(TaintedPathAdditionalTaintStep s).step(n1, n2)
73+
}
74+
}
75+
76+
/** Tracks flow from remote sources to the creation of a path. */
77+
module TaintedPathFlow = TaintTracking::Global<TaintedPathConfig>;
78+
79+
/**
80+
* A taint-tracking configuration for tracking flow from local user input to the creation of a path.
81+
*/
82+
module TaintedPathLocalConfig implements DataFlow::ConfigSig {
83+
predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
84+
85+
predicate isSink(DataFlow::Node sink) {
86+
sink.asExpr() = any(PathCreation p).getAnInput()
87+
or
88+
sinkNode(sink, "create-file")
89+
}
90+
91+
predicate isBarrier(DataFlow::Node sanitizer) {
92+
sanitizer.getType() instanceof BoxedType or
93+
sanitizer.getType() instanceof PrimitiveType or
94+
sanitizer.getType() instanceof NumberType or
95+
sanitizer instanceof PathInjectionSanitizer
96+
}
97+
98+
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
99+
any(TaintedPathAdditionalTaintStep s).step(n1, n2)
100+
}
101+
}
102+
103+
/** Tracks flow from local user input to the creation of a path. */
104+
module TaintedPathLocalFlow = TaintTracking::Global<TaintedPathLocalConfig>;

‎java/ql/lib/semmle/code/java/security/XssQuery.qll

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/** Provides a taint tracking configuration to track cross site scripting. */
2+
3+
import java
4+
import semmle.code.java.dataflow.FlowSources
5+
import semmle.code.java.dataflow.TaintTracking
6+
import semmle.code.java.security.XSS
7+
8+
/**
9+
* A taint-tracking configuration for cross site scripting vulnerabilities.
10+
*/
11+
module XssConfig implements DataFlow::ConfigSig {
12+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
13+
14+
predicate isSink(DataFlow::Node sink) { sink instanceof XssSink }
15+
16+
predicate isBarrier(DataFlow::Node node) { node instanceof XssSanitizer }
17+
18+
predicate isBarrierOut(DataFlow::Node node) { node instanceof XssSinkBarrier }
19+
20+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
21+
any(XssAdditionalTaintStep s).step(node1, node2)
22+
}
23+
}
24+
25+
/** Tracks flow from remote sources to cross site scripting vulnerabilities. */
26+
module XssFlow = TaintTracking::Global<XssConfig>;

‎java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/** Provides dataflow configurations to be used in ZipSlip queries. */
2+
3+
import java
4+
import semmle.code.java.dataflow.TaintTracking
5+
import semmle.code.java.security.PathSanitizer
6+
private import semmle.code.java.dataflow.ExternalFlow
7+
8+
/**
9+
* A method that returns the name of an archive entry.
10+
*/
11+
private class ArchiveEntryNameMethod extends Method {
12+
ArchiveEntryNameMethod() {
13+
exists(RefType archiveEntry |
14+
archiveEntry.hasQualifiedName("java.util.zip", "ZipEntry") or
15+
archiveEntry.hasQualifiedName("org.apache.commons.compress.archivers", "ArchiveEntry")
16+
|
17+
this.getDeclaringType().getAnAncestor() = archiveEntry and
18+
this.hasName("getName")
19+
)
20+
}
21+
}
22+
23+
/**
24+
* A taint-tracking configuration for reasoning about unsafe zip file extraction.
25+
*/
26+
module ZipSlipConfig implements DataFlow::ConfigSig {
27+
predicate isSource(DataFlow::Node source) {
28+
source.asExpr().(MethodAccess).getMethod() instanceof ArchiveEntryNameMethod
29+
}
30+
31+
predicate isSink(DataFlow::Node sink) { sink instanceof FileCreationSink }
32+
33+
predicate isBarrier(DataFlow::Node node) { node instanceof PathInjectionSanitizer }
34+
}
35+
36+
/** Tracks flow from archive entries to file creation. */
37+
module ZipSlipFlow = TaintTracking::Global<ZipSlipConfig>;
38+
39+
/**
40+
* A sink that represents a file creation, such as a file write, copy or move operation.
41+
*/
42+
private class FileCreationSink extends DataFlow::Node {
43+
FileCreationSink() { sinkNode(this, "create-file") }
44+
}

‎java/ql/src/Security/CWE/CWE-022/TaintedPath.ql

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,36 +14,8 @@
1414
*/
1515

1616
import java
17-
import semmle.code.java.dataflow.FlowSources
18-
private import semmle.code.java.dataflow.ExternalFlow
19-
import semmle.code.java.security.PathCreation
20-
import semmle.code.java.security.PathSanitizer
21-
import TaintedPathCommon
22-
23-
module TaintedPathConfig implements DataFlow::ConfigSig {
24-
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
25-
26-
predicate isSink(DataFlow::Node sink) {
27-
sink.asExpr() = any(PathCreation p).getAnInput()
28-
or
29-
sinkNode(sink, ["create-file", "read-file"])
30-
}
31-
32-
predicate isBarrier(DataFlow::Node sanitizer) {
33-
sanitizer.getType() instanceof BoxedType or
34-
sanitizer.getType() instanceof PrimitiveType or
35-
sanitizer.getType() instanceof NumberType or
36-
sanitizer instanceof PathInjectionSanitizer
37-
}
38-
39-
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
40-
any(TaintedPathAdditionalTaintStep s).step(n1, n2)
41-
}
42-
}
43-
44-
module TaintedPath = TaintTracking::Global<TaintedPathConfig>;
45-
46-
import TaintedPath::PathGraph
17+
import semmle.code.java.security.TaintedPathQuery
18+
import TaintedPathFlow::PathGraph
4719

4820
/**
4921
* Gets the data-flow node at which to report a path ending at `sink`.
@@ -53,13 +25,13 @@ import TaintedPath::PathGraph
5325
* continue to report there; otherwise we report directly at `sink`.
5426
*/
5527
DataFlow::Node getReportingNode(DataFlow::Node sink) {
56-
TaintedPath::flowTo(sink) and
28+
TaintedPathFlow::flowTo(sink) and
5729
if exists(PathCreation pc | pc.getAnInput() = sink.asExpr())
5830
then result.asExpr() = any(PathCreation pc | pc.getAnInput() = sink.asExpr())
5931
else result = sink
6032
}
6133

62-
from TaintedPath::PathNode source, TaintedPath::PathNode sink
63-
where TaintedPath::flowPath(source, sink)
34+
from TaintedPathFlow::PathNode source, TaintedPathFlow::PathNode sink
35+
where TaintedPathFlow::flowPath(source, sink)
6436
select getReportingNode(sink.getNode()), source, sink, "This path depends on a $@.",
6537
source.getNode(), "user-provided value"

0 commit comments

Comments
 (0)