Skip to content

Commit 50291c7

Browse files
committed
AtmConfig inherits from TaintTracking::Configuration.
That way the specific configs which inherit from `AtmConfig` also inherit from `TaintTracking::Configuration`. This removes the need for two separate config classes for each query.
1 parent 05a943c commit 50291c7

File tree

10 files changed

+90
-117
lines changed

10 files changed

+90
-117
lines changed

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/ATMConfig.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import EndpointCharacteristics as EndpointCharacteristics
2929
* `isAdditionalFlowStep` with a more generalised definition of additional edges. See
3030
* `NosqlInjectionATM.qll` for an example of doing this.
3131
*/
32-
abstract class AtmConfig extends string {
32+
abstract class AtmConfig extends JS::TaintTracking::Configuration {
3333
bindingset[this]
3434
AtmConfig() { any() }
3535

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/**
22
* For internal use only.
33
*
4+
* A taint-tracking configuration for reasoning about NoSQL injection vulnerabilities.
45
* Defines shared code used by the NoSQL injection boosted query.
56
*/
67

@@ -9,61 +10,20 @@ private import semmle.javascript.heuristics.SyntacticHeuristics
910
private import semmle.javascript.security.dataflow.NosqlInjectionCustomizations
1011
import AdaptiveThreatModeling
1112

12-
class NosqlInjectionAtmConfig extends AtmConfig {
13-
NosqlInjectionAtmConfig() { this = "NosqlInjectionATMConfig" }
13+
class Configuration extends AtmConfig {
14+
Configuration() { this = "NosqlInjectionATMConfig" }
1415

1516
override predicate isKnownSource(DataFlow::Node source) {
1617
source instanceof NosqlInjection::Source or TaintedObject::isSource(source, _)
1718
}
1819

1920
override EndpointType getASinkEndpointType() { result instanceof NosqlInjectionSinkType }
20-
}
21-
22-
/** DEPRECATED: Alias for NosqlInjectionAtmConfig */
23-
deprecated class NosqlInjectionATMConfig = NosqlInjectionAtmConfig;
2421

25-
/** Holds if src -> trg is an additional flow step in the non-boosted NoSql injection security query. */
26-
predicate isBaseAdditionalFlowStep(
27-
DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
28-
) {
29-
TaintedObject::step(src, trg, inlbl, outlbl)
30-
or
31-
// additional flow step to track taint through NoSQL query objects
32-
inlbl = TaintedObject::label() and
33-
outlbl = TaintedObject::label() and
34-
exists(NoSql::Query query, DataFlow::SourceNode queryObj |
35-
queryObj.flowsTo(query) and
36-
queryObj.flowsTo(trg) and
37-
src = queryObj.getAPropertyWrite().getRhs()
38-
)
39-
}
40-
41-
/**
42-
* Gets a value that is (transitively) written to `query`, where `query` is a NoSQL sink.
43-
*
44-
* This predicate allows us to propagate data flow through property writes and array constructors
45-
* within a query object, enabling the security query to pick up NoSQL injection vulnerabilities
46-
* involving more complex queries.
47-
*/
48-
DataFlow::Node getASubexpressionWithinQuery(DataFlow::Node query) {
49-
any(NosqlInjectionAtmConfig cfg).isEffectiveSink(query) and
50-
exists(DataFlow::SourceNode receiver |
51-
receiver = [getASubexpressionWithinQuery(query), query].getALocalSource()
52-
|
53-
result =
54-
[receiver.getAPropertyWrite().getRhs(), receiver.(DataFlow::ArrayCreationNode).getAnElement()]
55-
)
56-
}
57-
58-
/**
59-
* A taint-tracking configuration for reasoning about NoSQL injection vulnerabilities.
60-
*
61-
* This is largely a copy of the taint tracking configuration for the standard NoSQL injection
62-
* query, except additional ATM sinks have been added and the additional flow step has been
63-
* generalised to cover the sinks predicted by ATM.
64-
*/
65-
class Configuration extends TaintTracking::Configuration {
66-
Configuration() { this = "NosqlInjectionATM" }
22+
/*
23+
* This is largely a copy of the taint tracking configuration for the standard NoSQL injection
24+
* query, except additional ATM sinks have been added and the additional flow step has been
25+
* generalised to cover the sinks predicted by ATM.
26+
*/
6727

6828
override predicate isSource(DataFlow::Node source) { source instanceof NosqlInjection::Source }
6929

@@ -75,7 +35,7 @@ class Configuration extends TaintTracking::Configuration {
7535
sink.(NosqlInjection::Sink).getAFlowLabel() = label
7636
or
7737
// Allow effective sinks to have any taint label
78-
any(NosqlInjectionAtmConfig cfg).isEffectiveSink(sink)
38+
isEffectiveSink(sink)
7939
}
8040

8141
override predicate isSanitizer(DataFlow::Node node) {
@@ -94,7 +54,43 @@ class Configuration extends TaintTracking::Configuration {
9454
isBaseAdditionalFlowStep(src, trg, inlbl, outlbl)
9555
or
9656
// relaxed version of previous step to track taint through unmodeled NoSQL query objects
97-
any(NosqlInjectionAtmConfig cfg).isEffectiveSink(trg) and
57+
isEffectiveSink(trg) and
9858
src = getASubexpressionWithinQuery(trg)
9959
}
60+
61+
/** Holds if src -> trg is an additional flow step in the non-boosted NoSql injection security query. */
62+
private predicate isBaseAdditionalFlowStep(
63+
DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
64+
) {
65+
TaintedObject::step(src, trg, inlbl, outlbl)
66+
or
67+
// additional flow step to track taint through NoSQL query objects
68+
inlbl = TaintedObject::label() and
69+
outlbl = TaintedObject::label() and
70+
exists(NoSql::Query query, DataFlow::SourceNode queryObj |
71+
queryObj.flowsTo(query) and
72+
queryObj.flowsTo(trg) and
73+
src = queryObj.getAPropertyWrite().getRhs()
74+
)
75+
}
76+
77+
/**
78+
* Gets a value that is (transitively) written to `query`, where `query` is a NoSQL sink.
79+
*
80+
* This predicate allows us to propagate data flow through property writes and array constructors
81+
* within a query object, enabling the security query to pick up NoSQL injection vulnerabilities
82+
* involving more complex queries.
83+
*/
84+
private DataFlow::Node getASubexpressionWithinQuery(DataFlow::Node query) {
85+
isEffectiveSink(query) and
86+
exists(DataFlow::SourceNode receiver |
87+
receiver = [getASubexpressionWithinQuery(query), query].getALocalSource()
88+
|
89+
result =
90+
[
91+
receiver.getAPropertyWrite().getRhs(),
92+
receiver.(DataFlow::ArrayCreationNode).getAnElement()
93+
]
94+
)
95+
}
10096
}

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/SqlInjectionATM.qll

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,29 @@
11
/**
22
* For internal use only.
33
*
4+
* A taint-tracking configuration for reasoning about SQL injection vulnerabilities.
45
* Defines shared code used by the SQL injection boosted query.
56
*/
67

78
import semmle.javascript.heuristics.SyntacticHeuristics
89
import semmle.javascript.security.dataflow.SqlInjectionCustomizations
910
import AdaptiveThreatModeling
1011

11-
class SqlInjectionAtmConfig extends AtmConfig {
12-
SqlInjectionAtmConfig() { this = "SqlInjectionATMConfig" }
12+
class Configuration extends AtmConfig {
13+
Configuration() { this = "SqlInjectionATMConfig" }
1314

1415
override predicate isKnownSource(DataFlow::Node source) { source instanceof SqlInjection::Source }
1516

1617
override EndpointType getASinkEndpointType() { result instanceof SqlInjectionSinkType }
17-
}
18-
19-
/** DEPRECATED: Alias for SqlInjectionAtmConfig */
20-
deprecated class SqlInjectionATMConfig = SqlInjectionAtmConfig;
21-
22-
/**
23-
* A taint-tracking configuration for reasoning about SQL injection vulnerabilities.
24-
*
25-
* This is largely a copy of the taint tracking configuration for the standard SQL injection
26-
* query, except additional sinks have been added using the sink endpoint filter.
27-
*/
28-
class Configuration extends TaintTracking::Configuration {
29-
Configuration() { this = "SqlInjectionATM" }
3018

19+
/**
20+
* This is largely a copy of the taint tracking configuration for the standard SQL injection
21+
* query, except additional sinks have been added using the sink endpoint filter.
22+
*/
3123
override predicate isSource(DataFlow::Node source) { source instanceof SqlInjection::Source }
3224

3325
override predicate isSink(DataFlow::Node sink) {
34-
sink instanceof SqlInjection::Sink or any(SqlInjectionAtmConfig cfg).isEffectiveSink(sink)
26+
sink instanceof SqlInjection::Sink or isEffectiveSink(sink)
3527
}
3628

3729
override predicate isSanitizer(DataFlow::Node node) {

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/TaintedPathATM.qll

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,32 @@
11
/**
22
* For internal use only.
33
*
4+
* A taint-tracking configuration for reasoning about path injection vulnerabilities.
45
* Defines shared code used by the path injection boosted query.
56
*/
67

78
import semmle.javascript.heuristics.SyntacticHeuristics
89
import semmle.javascript.security.dataflow.TaintedPathCustomizations
910
import AdaptiveThreatModeling
1011

11-
class TaintedPathAtmConfig extends AtmConfig {
12-
TaintedPathAtmConfig() { this = "TaintedPathATMConfig" }
12+
class Configuration extends AtmConfig {
13+
Configuration() { this = "TaintedPathATMConfig" }
1314

1415
override predicate isKnownSource(DataFlow::Node source) { source instanceof TaintedPath::Source }
1516

1617
override EndpointType getASinkEndpointType() { result instanceof TaintedPathSinkType }
17-
}
18-
19-
/** DEPRECATED: Alias for TaintedPathAtmConfig */
20-
deprecated class TaintedPathATMConfig = TaintedPathAtmConfig;
21-
22-
/**
23-
* A taint-tracking configuration for reasoning about path injection vulnerabilities.
24-
*
25-
* This is largely a copy of the taint tracking configuration for the standard path injection
26-
* query, except additional ATM sinks have been added to the `isSink` predicate.
27-
*/
28-
class Configuration extends TaintTracking::Configuration {
29-
Configuration() { this = "TaintedPathATM" }
3018

19+
/**
20+
* This is largely a copy of the taint tracking configuration for the standard path injection
21+
* query, except additional ATM sinks have been added to the `isSink` predicate.
22+
*/
3123
override predicate isSource(DataFlow::Node source) { source instanceof TaintedPath::Source }
3224

3325
override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
3426
label = sink.(TaintedPath::Sink).getAFlowLabel()
3527
or
3628
// Allow effective sinks to have any taint label
37-
any(TaintedPathAtmConfig cfg).isEffectiveSink(sink)
29+
isEffectiveSink(sink)
3830
}
3931

4032
override predicate isSanitizer(DataFlow::Node node) { node instanceof TaintedPath::Sanitizer }
@@ -60,7 +52,7 @@ class Configuration extends TaintTracking::Configuration {
6052
* of barrier guards, we port the barrier guards for the boosted query from the standard library to
6153
* sanitizer guards here.
6254
*/
63-
class BarrierGuardNodeAsSanitizerGuardNode extends TaintTracking::LabeledSanitizerGuardNode {
55+
private class BarrierGuardNodeAsSanitizerGuardNode extends TaintTracking::LabeledSanitizerGuardNode {
6456
BarrierGuardNodeAsSanitizerGuardNode() { this instanceof TaintedPath::BarrierGuardNode }
6557

6658
override predicate sanitizes(boolean outcome, Expr e) {

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/XssATM.qll

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,31 +8,24 @@ private import semmle.javascript.heuristics.SyntacticHeuristics
88
private import semmle.javascript.security.dataflow.DomBasedXssCustomizations
99
import AdaptiveThreatModeling
1010

11-
class DomBasedXssAtmConfig extends AtmConfig {
12-
DomBasedXssAtmConfig() { this = "DomBasedXssATMConfig" }
11+
class Configuration extends AtmConfig {
12+
Configuration() { this = "DomBasedXssATMConfig" }
1313

1414
override predicate isKnownSource(DataFlow::Node source) { source instanceof DomBasedXss::Source }
1515

1616
override EndpointType getASinkEndpointType() { result instanceof XssSinkType }
17-
}
18-
19-
/** DEPRECATED: Alias for DomBasedXssAtmConfig */
20-
deprecated class DomBasedXssATMConfig = DomBasedXssAtmConfig;
21-
22-
/**
23-
* A taint-tracking configuration for reasoning about XSS vulnerabilities.
24-
*
25-
* This is largely a copy of the taint tracking configuration for the standard XSSThroughDom query,
26-
* except additional ATM sinks have been added to the `isSink` predicate.
27-
*/
28-
class Configuration extends TaintTracking::Configuration {
29-
Configuration() { this = "DomBasedXssATMConfiguration" }
3017

18+
/**
19+
* A taint-tracking configuration for reasoning about XSS vulnerabilities.
20+
*
21+
* This is largely a copy of the taint tracking configuration for the standard XSSThroughDom query,
22+
* except additional ATM sinks have been added to the `isSink` predicate.
23+
*/
3124
override predicate isSource(DataFlow::Node source) { source instanceof DomBasedXss::Source }
3225

3326
override predicate isSink(DataFlow::Node sink) {
3427
sink instanceof DomBasedXss::Sink or
35-
any(DomBasedXssAtmConfig cfg).isEffectiveSink(sink)
28+
isEffectiveSink(sink)
3629
}
3730

3831
override predicate isSanitizer(DataFlow::Node node) {

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/DebugResultInclusion.ql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,16 @@ private import experimental.adaptivethreatmodeling.XssATM as XssAtm
1919

2020
string getAReasonSinkExcluded(DataFlow::Node sinkCandidate, Query query) {
2121
query instanceof NosqlInjectionQuery and
22-
result = any(NosqlInjectionAtm::NosqlInjectionAtmConfig cfg).getAReasonSinkExcluded(sinkCandidate)
22+
result = any(NosqlInjectionAtm::Configuration cfg).getAReasonSinkExcluded(sinkCandidate)
2323
or
2424
query instanceof SqlInjectionQuery and
25-
result = any(SqlInjectionAtm::SqlInjectionAtmConfig cfg).getAReasonSinkExcluded(sinkCandidate)
25+
result = any(SqlInjectionAtm::Configuration cfg).getAReasonSinkExcluded(sinkCandidate)
2626
or
2727
query instanceof TaintedPathQuery and
28-
result = any(TaintedPathAtm::TaintedPathAtmConfig cfg).getAReasonSinkExcluded(sinkCandidate)
28+
result = any(TaintedPathAtm::Configuration cfg).getAReasonSinkExcluded(sinkCandidate)
2929
or
3030
query instanceof XssQuery and
31-
result = any(XssAtm::DomBasedXssAtmConfig cfg).getAReasonSinkExcluded(sinkCandidate)
31+
result = any(XssAtm::Configuration cfg).getAReasonSinkExcluded(sinkCandidate)
3232
}
3333

3434
pragma[inline]

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractEndpointMapping.ql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,15 @@ from string queryName, AtmConfig c, EndpointType e
1414
where
1515
(
1616
queryName = "SqlInjection" and
17-
c instanceof SqlInjectionAtm::SqlInjectionAtmConfig
17+
c instanceof SqlInjectionAtm::Configuration
1818
or
1919
queryName = "NosqlInjection" and
20-
c instanceof NosqlInjectionAtm::NosqlInjectionAtmConfig
20+
c instanceof NosqlInjectionAtm::Configuration
2121
or
2222
queryName = "TaintedPath" and
23-
c instanceof TaintedPathAtm::TaintedPathAtmConfig
23+
c instanceof TaintedPathAtm::Configuration
2424
or
25-
queryName = "Xss" and c instanceof XssAtm::DomBasedXssAtmConfig
25+
queryName = "Xss" and c instanceof XssAtm::Configuration
2626
) and
2727
e = c.getASinkEndpointType()
2828
select queryName, e.getEncoding() as label

javascript/ql/experimental/adaptivethreatmodeling/test/endpoint_large_scale/EndpointFeatures.ql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ private import experimental.adaptivethreatmodeling.EndpointCharacteristics as En
1717

1818
query predicate tokenFeatures(DataFlow::Node endpoint, string featureName, string featureValue) {
1919
(
20-
not exists(any(NosqlInjectionAtm::NosqlInjectionAtmConfig cfg).getAReasonSinkExcluded(endpoint)) or
21-
not exists(any(SqlInjectionAtm::SqlInjectionAtmConfig cfg).getAReasonSinkExcluded(endpoint)) or
22-
not exists(any(TaintedPathAtm::TaintedPathAtmConfig cfg).getAReasonSinkExcluded(endpoint)) or
23-
not exists(any(XssAtm::DomBasedXssAtmConfig cfg).getAReasonSinkExcluded(endpoint)) or
20+
not exists(any(NosqlInjectionAtm::Configuration cfg).getAReasonSinkExcluded(endpoint)) or
21+
not exists(any(SqlInjectionAtm::Configuration cfg).getAReasonSinkExcluded(endpoint)) or
22+
not exists(any(TaintedPathAtm::Configuration cfg).getAReasonSinkExcluded(endpoint)) or
23+
not exists(any(XssAtm::Configuration cfg).getAReasonSinkExcluded(endpoint)) or
2424
any(EndpointCharacteristics::IsArgumentToModeledFunctionCharacteristic characteristic)
2525
.getEndpoints(endpoint)
2626
) and

javascript/ql/experimental/adaptivethreatmodeling/test/endpoint_large_scale/FilteredTruePositives.ql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,24 +23,24 @@ import experimental.adaptivethreatmodeling.XssATM as XssAtm
2323

2424
query predicate nosqlFilteredTruePositives(DataFlow::Node endpoint, string reason) {
2525
endpoint instanceof NosqlInjection::Sink and
26-
reason = any(NosqlInjectionAtm::NosqlInjectionAtmConfig cfg).getAReasonSinkExcluded(endpoint) and
26+
reason = any(NosqlInjectionAtm::Configuration cfg).getAReasonSinkExcluded(endpoint) and
2727
not reason = ["argument to modeled function", "modeled sink", "modeled database access"]
2828
}
2929

3030
query predicate sqlFilteredTruePositives(DataFlow::Node endpoint, string reason) {
3131
endpoint instanceof SqlInjection::Sink and
32-
reason = any(SqlInjectionAtm::SqlInjectionAtmConfig cfg).getAReasonSinkExcluded(endpoint) and
32+
reason = any(SqlInjectionAtm::Configuration cfg).getAReasonSinkExcluded(endpoint) and
3333
reason != "argument to modeled function"
3434
}
3535

3636
query predicate taintedPathFilteredTruePositives(DataFlow::Node endpoint, string reason) {
3737
endpoint instanceof TaintedPath::Sink and
38-
reason = any(TaintedPathAtm::TaintedPathAtmConfig cfg).getAReasonSinkExcluded(endpoint) and
38+
reason = any(TaintedPathAtm::Configuration cfg).getAReasonSinkExcluded(endpoint) and
3939
reason != "argument to modeled function"
4040
}
4141

4242
query predicate xssFilteredTruePositives(DataFlow::Node endpoint, string reason) {
4343
endpoint instanceof DomBasedXss::Sink and
44-
reason = any(XssAtm::DomBasedXssAtmConfig cfg).getAReasonSinkExcluded(endpoint) and
44+
reason = any(XssAtm::Configuration cfg).getAReasonSinkExcluded(endpoint) and
4545
reason != "argument to modeled function"
4646
}

javascript/ql/experimental/adaptivethreatmodeling/test/modeled_apis/nosql_endpoint_filter_ignores_modeled_apis.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@ import javascript
22
import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInjectionAtm
33

44
query predicate effectiveSinks(DataFlow::Node node) {
5-
not exists(any(NosqlInjectionAtm::NosqlInjectionAtmConfig cfg).getAReasonSinkExcluded(node))
5+
not exists(any(NosqlInjectionAtm::Configuration cfg).getAReasonSinkExcluded(node))
66
}

0 commit comments

Comments
 (0)