Skip to content

Commit b102dda

Browse files
egregius313egregius313
authored
Merge pull request github#12542 from egregius313/egregius313/refactor-more-queries-to-dataflow-module-api
Java: Refactor more queries to the new DataFlow module API (part 2)
2 parents 574b220 + b64ca5d commit b102dda

13 files changed

+130
-106
lines changed

‎java/ql/src/Security/CWE/CWE-078/ExecTaintedLocal.ql

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,13 @@ import semmle.code.java.Expr
1616
import semmle.code.java.dataflow.FlowSources
1717
import semmle.code.java.security.ExternalProcess
1818
import semmle.code.java.security.CommandArguments
19-
import DataFlow::PathGraph
2019

21-
class LocalUserInputToArgumentToExecFlowConfig extends TaintTracking::Configuration {
22-
LocalUserInputToArgumentToExecFlowConfig() { this = "LocalUserInputToArgumentToExecFlowConfig" }
20+
module LocalUserInputToArgumentToExecFlowConfig implements DataFlow::ConfigSig {
21+
predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
2322

24-
override predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
23+
predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof ArgumentToExec }
2524

26-
override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof ArgumentToExec }
27-
28-
override predicate isSanitizer(DataFlow::Node node) {
25+
predicate isBarrier(DataFlow::Node node) {
2926
node.getType() instanceof PrimitiveType
3027
or
3128
node.getType() instanceof BoxedType
@@ -34,9 +31,16 @@ class LocalUserInputToArgumentToExecFlowConfig extends TaintTracking::Configurat
3431
}
3532
}
3633

34+
module LocalUserInputToArgumentToExecFlow =
35+
TaintTracking::Make<LocalUserInputToArgumentToExecFlowConfig>;
36+
37+
import LocalUserInputToArgumentToExecFlow::PathGraph
38+
3739
from
38-
DataFlow::PathNode source, DataFlow::PathNode sink, ArgumentToExec execArg,
39-
LocalUserInputToArgumentToExecFlowConfig conf
40-
where conf.hasFlowPath(source, sink) and sink.getNode().asExpr() = execArg
40+
LocalUserInputToArgumentToExecFlow::PathNode source,
41+
LocalUserInputToArgumentToExecFlow::PathNode sink, ArgumentToExec execArg
42+
where
43+
LocalUserInputToArgumentToExecFlow::hasFlowPath(source, sink) and
44+
sink.getNode().asExpr() = execArg
4145
select execArg, source, sink, "This command line depends on a $@.", source.getNode(),
4246
"user-provided value"

‎java/ql/src/Security/CWE/CWE-079/XSSLocal.ql

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,18 @@
1414
import java
1515
import semmle.code.java.dataflow.FlowSources
1616
import semmle.code.java.security.XSS
17-
import DataFlow::PathGraph
1817

19-
class XssLocalConfig extends TaintTracking::Configuration {
20-
XssLocalConfig() { this = "XSSLocalConfig" }
18+
module XssLocalConfig implements DataFlow::ConfigSig {
19+
predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
2120

22-
override predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
23-
24-
override predicate isSink(DataFlow::Node sink) { sink instanceof XssSink }
21+
predicate isSink(DataFlow::Node sink) { sink instanceof XssSink }
2522
}
2623

27-
from DataFlow::PathNode source, DataFlow::PathNode sink, XssLocalConfig conf
28-
where conf.hasFlowPath(source, sink)
24+
module XssLocalFlow = TaintTracking::Make<XssLocalConfig>;
25+
26+
import XssLocalFlow::PathGraph
27+
28+
from XssLocalFlow::PathNode source, XssLocalFlow::PathNode sink
29+
where XssLocalFlow::hasFlowPath(source, sink)
2930
select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to $@.",
3031
source.getNode(), "user-provided value"

‎java/ql/src/Security/CWE/CWE-089/SqlConcatenated.ql

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,28 +25,27 @@ class UncontrolledStringBuilderSource extends DataFlow::ExprNode {
2525
}
2626
}
2727

28-
class UncontrolledStringBuilderSourceFlowConfig extends TaintTracking::Configuration {
29-
UncontrolledStringBuilderSourceFlowConfig() {
30-
this = "SqlConcatenated::UncontrolledStringBuilderSourceFlowConfig"
31-
}
32-
33-
override predicate isSource(DataFlow::Node src) { src instanceof UncontrolledStringBuilderSource }
28+
module UncontrolledStringBuilderSourceFlowConfig implements DataFlow::ConfigSig {
29+
predicate isSource(DataFlow::Node src) { src instanceof UncontrolledStringBuilderSource }
3430

35-
override predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink }
31+
predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink }
3632

37-
override predicate isSanitizer(DataFlow::Node node) {
33+
predicate isBarrier(DataFlow::Node node) {
3834
node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
3935
}
4036
}
4137

38+
module UncontrolledStringBuilderSourceFlow =
39+
TaintTracking::Make<UncontrolledStringBuilderSourceFlowConfig>;
40+
4241
from QueryInjectionSink query, Expr uncontrolled
4342
where
4443
(
4544
builtFromUncontrolledConcat(query.asExpr(), uncontrolled)
4645
or
47-
exists(StringBuilderVar sbv, UncontrolledStringBuilderSourceFlowConfig conf |
46+
exists(StringBuilderVar sbv |
4847
uncontrolledStringBuilderQuery(sbv, uncontrolled) and
49-
conf.hasFlow(DataFlow::exprNode(sbv.getToStringCall()), query)
48+
UncontrolledStringBuilderSourceFlow::hasFlow(DataFlow::exprNode(sbv.getToStringCall()), query)
5049
)
5150
) and
5251
not queryTaintedBy(query, _, _)

‎java/ql/src/Security/CWE/CWE-089/SqlTaintedLocal.ql

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,29 @@
1515
import semmle.code.java.Expr
1616
import semmle.code.java.dataflow.FlowSources
1717
import semmle.code.java.security.SqlInjectionQuery
18-
import DataFlow::PathGraph
1918

20-
class LocalUserInputToQueryInjectionFlowConfig extends TaintTracking::Configuration {
21-
LocalUserInputToQueryInjectionFlowConfig() { this = "LocalUserInputToQueryInjectionFlowConfig" }
19+
module LocalUserInputToQueryInjectionFlowConfig implements DataFlow::ConfigSig {
20+
predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
2221

23-
override predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
22+
predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink }
2423

25-
override predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink }
26-
27-
override predicate isSanitizer(DataFlow::Node node) {
24+
predicate isBarrier(DataFlow::Node node) {
2825
node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
2926
}
3027

31-
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
28+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
3229
any(AdditionalQueryInjectionTaintStep s).step(node1, node2)
3330
}
3431
}
3532

33+
module LocalUserInputToQueryInjectionFlow =
34+
TaintTracking::Make<LocalUserInputToQueryInjectionFlowConfig>;
35+
36+
import LocalUserInputToQueryInjectionFlow::PathGraph
37+
3638
from
37-
DataFlow::PathNode source, DataFlow::PathNode sink, LocalUserInputToQueryInjectionFlowConfig conf
38-
where conf.hasFlowPath(source, sink)
39+
LocalUserInputToQueryInjectionFlow::PathNode source,
40+
LocalUserInputToQueryInjectionFlow::PathNode sink
41+
where LocalUserInputToQueryInjectionFlow::hasFlowPath(source, sink)
3942
select sink.getNode(), source, sink, "This query depends on a $@.", source.getNode(),
4043
"user-provided value"

‎java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstruction.ql

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,25 +13,28 @@
1313
import java
1414
import ArraySizing
1515
import semmle.code.java.dataflow.FlowSources
16-
import DataFlow::PathGraph
1716

18-
class Conf extends TaintTracking::Configuration {
19-
Conf() { this = "RemoteUserInputTocanThrowOutOfBoundsDueToEmptyArrayConfig" }
17+
private module ImproperValidationOfArrayConstructionConfig implements DataFlow::ConfigSig {
18+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
2019

21-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
22-
23-
override predicate isSink(DataFlow::Node sink) {
20+
predicate isSink(DataFlow::Node sink) {
2421
any(CheckableArrayAccess caa).canThrowOutOfBoundsDueToEmptyArray(sink.asExpr(), _)
2522
}
2623
}
2724

25+
module ImproperValidationOfArrayConstructionFlow =
26+
TaintTracking::Make<ImproperValidationOfArrayConstructionConfig>;
27+
28+
import ImproperValidationOfArrayConstructionFlow::PathGraph
29+
2830
from
29-
DataFlow::PathNode source, DataFlow::PathNode sink, Expr sizeExpr,
31+
ImproperValidationOfArrayConstructionFlow::PathNode source,
32+
ImproperValidationOfArrayConstructionFlow::PathNode sink, Expr sizeExpr,
3033
ArrayCreationExpr arrayCreation, CheckableArrayAccess arrayAccess
3134
where
3235
arrayAccess.canThrowOutOfBoundsDueToEmptyArray(sizeExpr, arrayCreation) and
3336
sizeExpr = sink.getNode().asExpr() and
34-
any(Conf conf).hasFlowPath(source, sink)
37+
ImproperValidationOfArrayConstructionFlow::hasFlowPath(source, sink)
3538
select arrayAccess.getIndexExpr(), source, sink,
3639
"This accesses the $@, but the array is initialized using a $@ which may be zero.", arrayCreation,
3740
"array", source.getNode(), "user-provided value"

‎java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstructionCodeSpecified.ql

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

1414
import java
1515
import ArraySizing
16-
import DataFlow::PathGraph
16+
import semmle.code.java.dataflow.TaintTracking
1717

18-
class BoundedFlowSourceConf extends DataFlow::Configuration {
19-
BoundedFlowSourceConf() { this = "BoundedFlowSource" }
20-
21-
override predicate isSource(DataFlow::Node source) {
18+
module BoundedFlowSourceConfig implements DataFlow::ConfigSig {
19+
predicate isSource(DataFlow::Node source) {
2220
source instanceof BoundedFlowSource and
2321
// There is not a fixed lower bound which is greater than zero.
2422
not source.(BoundedFlowSource).lowerBound() > 0
2523
}
2624

27-
override predicate isSink(DataFlow::Node sink) {
25+
predicate isSink(DataFlow::Node sink) {
2826
any(CheckableArrayAccess caa).canThrowOutOfBoundsDueToEmptyArray(sink.asExpr(), _)
2927
}
3028
}
3129

30+
module BoundedFlowSourceFlow = DataFlow::Make<BoundedFlowSourceConfig>;
31+
32+
import BoundedFlowSourceFlow::PathGraph
33+
3234
from
33-
DataFlow::PathNode source, DataFlow::PathNode sink, BoundedFlowSource boundedsource,
34-
Expr sizeExpr, ArrayCreationExpr arrayCreation, CheckableArrayAccess arrayAccess
35+
BoundedFlowSourceFlow::PathNode source, BoundedFlowSourceFlow::PathNode sink,
36+
BoundedFlowSource boundedsource, Expr sizeExpr, ArrayCreationExpr arrayCreation,
37+
CheckableArrayAccess arrayAccess
3538
where
3639
arrayAccess.canThrowOutOfBoundsDueToEmptyArray(sizeExpr, arrayCreation) and
3740
sizeExpr = sink.getNode().asExpr() and
3841
boundedsource = source.getNode() and
39-
any(BoundedFlowSourceConf conf).hasFlowPath(source, sink)
42+
BoundedFlowSourceFlow::hasFlowPath(source, sink)
4043
select arrayAccess.getIndexExpr(), source, sink,
4144
"This accesses the $@, but the array is initialized using $@ which may be zero.", arrayCreation,
4245
"array", boundedsource, boundedsource.getDescription().toLowerCase()

‎java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstructionLocal.ql

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,25 +14,28 @@
1414
import java
1515
import ArraySizing
1616
import semmle.code.java.dataflow.FlowSources
17-
import DataFlow::PathGraph
1817

19-
class Conf extends TaintTracking::Configuration {
20-
Conf() { this = "LocalUserInputTocanThrowOutOfBoundsDueToEmptyArrayConfig" }
18+
module ImproperValidationOfArrayConstructionLocalConfig implements DataFlow::ConfigSig {
19+
predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
2120

22-
override predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
23-
24-
override predicate isSink(DataFlow::Node sink) {
21+
predicate isSink(DataFlow::Node sink) {
2522
any(CheckableArrayAccess caa).canThrowOutOfBoundsDueToEmptyArray(sink.asExpr(), _)
2623
}
2724
}
2825

26+
module ImproperValidationOfArrayConstructionLocalFlow =
27+
TaintTracking::Make<ImproperValidationOfArrayConstructionLocalConfig>;
28+
29+
import ImproperValidationOfArrayConstructionLocalFlow::PathGraph
30+
2931
from
30-
DataFlow::PathNode source, DataFlow::PathNode sink, Expr sizeExpr,
32+
ImproperValidationOfArrayConstructionLocalFlow::PathNode source,
33+
ImproperValidationOfArrayConstructionLocalFlow::PathNode sink, Expr sizeExpr,
3134
ArrayCreationExpr arrayCreation, CheckableArrayAccess arrayAccess
3235
where
3336
arrayAccess.canThrowOutOfBoundsDueToEmptyArray(sizeExpr, arrayCreation) and
3437
sizeExpr = sink.getNode().asExpr() and
35-
any(Conf conf).hasFlowPath(source, sink)
38+
ImproperValidationOfArrayConstructionLocalFlow::hasFlowPath(source, sink)
3639
select arrayAccess.getIndexExpr(), source, sink,
3740
"This accesses the $@, but the array is initialized using a $@ which may be zero.", arrayCreation,
3841
"array", source.getNode(), "user-provided value"

‎java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndex.ql

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,24 +13,28 @@
1313
import java
1414
import ArraySizing
1515
import semmle.code.java.dataflow.FlowSources
16-
import DataFlow::PathGraph
1716

18-
class Conf extends TaintTracking::Configuration {
19-
Conf() { this = "RemoteUserInputTocanThrowOutOfBoundsDueToEmptyArrayConfig" }
17+
module ImproperValidationOfArrayIndexConfig implements DataFlow::ConfigSig {
18+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
2019

21-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
22-
23-
override predicate isSink(DataFlow::Node sink) {
20+
predicate isSink(DataFlow::Node sink) {
2421
any(CheckableArrayAccess caa).canThrowOutOfBounds(sink.asExpr())
2522
}
2623

27-
override predicate isSanitizer(DataFlow::Node node) { node.getType() instanceof BooleanType }
24+
predicate isBarrier(DataFlow::Node node) { node.getType() instanceof BooleanType }
2825
}
2926

30-
from DataFlow::PathNode source, DataFlow::PathNode sink, CheckableArrayAccess arrayAccess
27+
module ImproperValidationOfArrayIndexFlow =
28+
TaintTracking::Make<ImproperValidationOfArrayIndexConfig>;
29+
30+
import ImproperValidationOfArrayIndexFlow::PathGraph
31+
32+
from
33+
ImproperValidationOfArrayIndexFlow::PathNode source,
34+
ImproperValidationOfArrayIndexFlow::PathNode sink, CheckableArrayAccess arrayAccess
3135
where
3236
arrayAccess.canThrowOutOfBounds(sink.getNode().asExpr()) and
33-
any(Conf conf).hasFlowPath(source, sink)
37+
ImproperValidationOfArrayIndexFlow::hasFlowPath(source, sink)
3438
select arrayAccess.getIndexExpr(), source, sink,
3539
"This index depends on a $@ which can cause an ArrayIndexOutOfBoundsException.", source.getNode(),
3640
"user-provided value"

‎java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndexCodeSpecified.ql

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,25 +14,27 @@
1414
import java
1515
import ArraySizing
1616
import BoundingChecks
17-
import DataFlow::PathGraph
17+
import semmle.code.java.dataflow.TaintTracking
1818

19-
class BoundedFlowSourceConf extends DataFlow::Configuration {
20-
BoundedFlowSourceConf() { this = "BoundedFlowSource" }
19+
module BoundedFlowSourceConfig implements DataFlow::ConfigSig {
20+
predicate isSource(DataFlow::Node source) { source instanceof BoundedFlowSource }
2121

22-
override predicate isSource(DataFlow::Node source) { source instanceof BoundedFlowSource }
23-
24-
override predicate isSink(DataFlow::Node sink) {
22+
predicate isSink(DataFlow::Node sink) {
2523
exists(CheckableArrayAccess arrayAccess | arrayAccess.canThrowOutOfBounds(sink.asExpr()))
2624
}
2725
}
2826

27+
module BoundedFlowSourceFlow = DataFlow::Make<BoundedFlowSourceConfig>;
28+
29+
import BoundedFlowSourceFlow::PathGraph
30+
2931
from
30-
DataFlow::PathNode source, DataFlow::PathNode sink, BoundedFlowSource boundedsource,
31-
CheckableArrayAccess arrayAccess
32+
BoundedFlowSourceFlow::PathNode source, BoundedFlowSourceFlow::PathNode sink,
33+
BoundedFlowSource boundedsource, CheckableArrayAccess arrayAccess
3234
where
3335
arrayAccess.canThrowOutOfBounds(sink.getNode().asExpr()) and
3436
boundedsource = source.getNode() and
35-
any(BoundedFlowSourceConf conf).hasFlowPath(source, sink) and
37+
BoundedFlowSourceFlow::hasFlowPath(source, sink) and
3638
boundedsource != sink.getNode() and
3739
not (
3840
(

‎java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndexLocal.ql

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,26 @@
1414
import java
1515
import ArraySizing
1616
import semmle.code.java.dataflow.FlowSources
17-
import DataFlow::PathGraph
1817

19-
class Conf extends TaintTracking::Configuration {
20-
Conf() { this = "LocalUserInputTocanThrowOutOfBoundsDueToEmptyArrayConfig" }
18+
module ImproperValidationOfArrayIndexLocalConfig implements DataFlow::ConfigSig {
19+
predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
2120

22-
override predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
23-
24-
override predicate isSink(DataFlow::Node sink) {
21+
predicate isSink(DataFlow::Node sink) {
2522
any(CheckableArrayAccess caa).canThrowOutOfBounds(sink.asExpr())
2623
}
2724
}
2825

29-
from DataFlow::PathNode source, DataFlow::PathNode sink, CheckableArrayAccess arrayAccess
26+
module ImproperValidationOfArrayIndexLocalFlow =
27+
TaintTracking::Make<ImproperValidationOfArrayIndexLocalConfig>;
28+
29+
import ImproperValidationOfArrayIndexLocalFlow::PathGraph
30+
31+
from
32+
ImproperValidationOfArrayIndexLocalFlow::PathNode source,
33+
ImproperValidationOfArrayIndexLocalFlow::PathNode sink, CheckableArrayAccess arrayAccess
3034
where
3135
arrayAccess.canThrowOutOfBounds(sink.getNode().asExpr()) and
32-
any(Conf conf).hasFlowPath(source, sink)
36+
ImproperValidationOfArrayIndexLocalFlow::hasFlowPath(source, sink)
3337
select arrayAccess.getIndexExpr(), source, sink,
3438
"This index depends on a $@ which can cause an ArrayIndexOutOfBoundsException.", source.getNode(),
3539
"user-provided value"

0 commit comments

Comments
 (0)