Skip to content

Commit bdf29d0

Browse files
author
Max Schaefer
committed
JavaScript: Allow summary details to be omitted.
If a summary does not specify a configuration, it is taken to apply to all configurations without custom sanitisers/barriers. If a source summary does not specify a flow label, `data` is assumed. If a sink summary does not specify a flow label, both `data` and `taint` are assumed. Flow step summaries cannot omit flow labels. Note that the standard extraction queries always provide explicit configurations and flow labels, and hence do not exercise this functionality.
1 parent 7c87c43 commit bdf29d0

3 files changed

Lines changed: 79 additions & 19 deletions

File tree

javascript/ql/src/Security/Summaries/ImportFromCsv.qll

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import javascript
77
import semmle.javascript.dataflow.Portals
88
import external.ExternalArtifact
9+
private import Shared
910

1011
/**
1112
* An additional source specified in an `additional-sources.csv` file.
@@ -26,18 +27,25 @@ class AdditionalSourceSpec extends ExternalData {
2627
* Gets the flow label of this source.
2728
*/
2829
DataFlow::FlowLabel getFlowLabel() {
29-
result.toString() = getField(1)
30+
sourceFlowLabelSpec(result, getField(1))
3031
}
3132

3233
/**
33-
* Gets the configuration for which this is a source.
34+
* Gets the configuration for which this is a source, or any
35+
* configuration if this source does not specify a configuration.
3436
*/
3537
DataFlow::Configuration getConfiguration() {
36-
result.toString() = getField(2)
38+
configSpec(result, getField(2))
3739
}
3840

3941
override string toString() {
40-
result = getPortal() + " as " + getFlowLabel() + " source for " + getConfiguration()
42+
exists (string config |
43+
if getField(2) = "" then
44+
config = "any configuration"
45+
else
46+
config = getConfiguration() |
47+
result = getPortal() + " as " + getFlowLabel() + " source for " + config
48+
)
4149
}
4250
}
4351

@@ -69,21 +77,30 @@ class AdditionalSinkSpec extends ExternalData {
6977
}
7078

7179
/**
72-
* Gets the flow label of this sink.
80+
* Gets the flow label of this sink, or any standard flow label if this sink
81+
* does not specify a flow label.
7382
*/
7483
DataFlow::FlowLabel getFlowLabel() {
75-
result.toString() = getField(1)
84+
sinkFlowLabelSpec(result, getField(1))
7685
}
7786

7887
/**
79-
* Gets the configuration for which this is a sink.
88+
* Gets the configuration for which this is a sink, or any configuration if
89+
* this sink does not specify a configuration.
8090
*/
8191
DataFlow::Configuration getConfiguration() {
82-
result.toString() = getField(2)
92+
configSpec(result, getField(2))
8393
}
8494

8595
override string toString() {
86-
result = getPortal() + " as " + getFlowLabel() + " sink for " + getConfiguration()
96+
exists (string labels, string config |
97+
labels = strictconcat(getFlowLabel(), " or ") and
98+
if getField(2) = "" then
99+
config = "any configuration"
100+
else
101+
config = getConfiguration() |
102+
result = getPortal() + " as " + labels + " sink for " + config
103+
)
87104
}
88105
}
89106

@@ -138,13 +155,19 @@ class AdditionalStepSpec extends ExternalData {
138155
* Gets the configuration to which this step should be added.
139156
*/
140157
DataFlow::Configuration getConfiguration() {
141-
result.toString() = getField(4)
158+
configSpec(result, getField(4))
142159
}
143160

144161
override string toString() {
145-
result = "edge from " + getStartPortal() + " to " + getEndPortal() +
146-
", transforming " + getStartFlowLabel() + " into " + getEndFlowLabel() +
147-
" for " + getConfiguration()
162+
exists (string config |
163+
if getField(4) = "" then
164+
config = "any configuration"
165+
else
166+
config = getConfiguration() |
167+
result = "edge from " + getStartPortal() + " to " + getEndPortal() +
168+
", transforming " + getStartFlowLabel() + " into " + getEndFlowLabel() +
169+
" for " + config
170+
)
148171
}
149172
}
150173

javascript/ql/src/Security/Summaries/ImportFromExternalPredicates.qll

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import javascript
77
import semmle.javascript.dataflow.Portals
88
import external.ExternalArtifact
9+
import Shared
910

1011
/**
1112
* An external predicate providing information about additional sources.
@@ -42,8 +43,7 @@ private class AdditionalSourceFromSpec extends DataFlow::AdditionalSource {
4243
}
4344

4445
override predicate isSourceFor(DataFlow::Configuration cfg, DataFlow::FlowLabel lbl) {
45-
cfg.toString() = config and
46-
lbl = flowLabel
46+
configSpec(cfg, config) and sourceFlowLabelSpec(lbl, flowLabel)
4747
}
4848
}
4949

@@ -61,8 +61,7 @@ private class AdditionalSinkFromSpec extends DataFlow::AdditionalSink {
6161
}
6262

6363
override predicate isSinkFor(DataFlow::Configuration cfg, DataFlow::FlowLabel lbl) {
64-
cfg.toString() = config and
65-
lbl = flowLabel
64+
configSpec(cfg, config) and sinkFlowLabelSpec(lbl, flowLabel)
6665
}
6766
}
6867
/**
@@ -75,8 +74,9 @@ private class AdditionalFlowStepFromSpec extends DataFlow::Configuration {
7574
string endFlowLabel;
7675

7776
AdditionalFlowStepFromSpec() {
78-
exists (Portal startPortal, Portal endPortal |
79-
additionalSteps(startPortal.toString(), startFlowLabel, endPortal.toString(), endFlowLabel, this) and
77+
exists (Portal startPortal, Portal endPortal, string config |
78+
additionalSteps(startPortal.toString(), startFlowLabel, endPortal.toString(), endFlowLabel, config) and
79+
configSpec(this, config) and
8080
entry = startPortal.getAnEntryNode(_) and
8181
exit = endPortal.getAnExitNode(_)
8282
)
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/**
2+
* Provides utility predicates for working with flow summary specifications.
3+
*/
4+
5+
import javascript
6+
7+
/**
8+
* Holds if `config` matches `spec`, that is, either `spec` is the name of `config`
9+
* or `spec` is the empty string and `config` is an arbitrary configuration.
10+
*/
11+
predicate configSpec(DataFlow::Configuration config, string spec) {
12+
config.toString() = spec
13+
or
14+
spec = ""
15+
}
16+
17+
/**
18+
* Holds if `lbl` matches `spec`, that is, either `spec` is the name of `config`
19+
* or `spec` is the empty string and `lbl` is the built-in flow label `data`.
20+
*/
21+
predicate sourceFlowLabelSpec(DataFlow::FlowLabel lbl, string spec) {
22+
lbl.toString() = spec
23+
or
24+
spec = "" and
25+
lbl = DataFlow::FlowLabel::data()
26+
}
27+
28+
/**
29+
* Holds if `lbl` matches `spec`, that is, either `spec` is the name of `config`
30+
* or `spec` is the empty string and `lbl` is an arbitrary standard flow label.
31+
*/
32+
predicate sinkFlowLabelSpec(DataFlow::FlowLabel lbl, string spec) {
33+
lbl.toString() = spec
34+
or
35+
spec = "" and
36+
lbl instanceof DataFlow::StandardFlowLabel
37+
}

0 commit comments

Comments
 (0)