Skip to content

Commit a44bdf3

Browse files
committed
JS: Generate summaries from summaryModel, and only generate steps as a fallback
1 parent 0fc1ae2 commit a44bdf3

File tree

3 files changed

+48
-4
lines changed

3 files changed

+48
-4
lines changed

javascript/ql/lib/semmle/javascript/dataflow/internal/FlowSummaryPrivate.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,3 +264,7 @@ module Stage {
264264
cached
265265
predicate backref() { optionalStep(_, _, _) }
266266
}
267+
268+
predicate unsupportedCallable = Private::unsupportedCallable/1;
269+
270+
predicate unsupportedCallable = Private::unsupportedCallable/4;

javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
private import javascript
2020
private import internal.ApiGraphModels as Shared
2121
private import internal.ApiGraphModelsSpecific as Specific
22+
private import semmle.javascript.dataflow.internal.FlowSummaryPrivate
2223
private import semmle.javascript.endpoints.EndpointNaming as EndpointNaming
2324
import Shared::ModelInput as ModelInput
2425
import Shared::ModelOutput as ModelOutput
@@ -45,12 +46,50 @@ private class ThreatModelSourceFromDataExtension extends ThreatModelSource::Rang
4546
}
4647
}
4748

49+
private class SummarizedCallableFromModel extends DataFlow::SummarizedCallable {
50+
string type;
51+
string path;
52+
53+
SummarizedCallableFromModel() {
54+
ModelOutput::relevantSummaryModel(type, path, _, _, _, _) and
55+
this = type + ";" + path
56+
}
57+
58+
override DataFlow::InvokeNode getACall() { ModelOutput::resolvedSummaryBase(type, path, result) }
59+
60+
override predicate propagatesFlow(
61+
string input, string output, boolean preservesValue, string model
62+
) {
63+
exists(string kind | ModelOutput::relevantSummaryModel(type, path, input, output, kind, model) |
64+
kind = "value" and
65+
preservesValue = true
66+
or
67+
kind = "taint" and
68+
preservesValue = false
69+
)
70+
}
71+
72+
predicate hasTypeAndPath(string type_, string path_) { type = type_ and path = path_ }
73+
74+
predicate isUnsupportedByFlowSummaries() { unsupportedCallable(this) }
75+
}
76+
77+
private predicate shouldInduceStepsFromSummary(string type, string path) {
78+
exists(SummarizedCallableFromModel callable |
79+
callable.isUnsupportedByFlowSummaries() and
80+
callable.hasTypeAndPath(type, path)
81+
)
82+
}
83+
4884
/**
4985
* Holds if `path` is an input or output spec for a summary with the given `base` node.
5086
*/
5187
pragma[nomagic]
5288
private predicate relevantInputOutputPath(API::InvokeNode base, AccessPath inputOrOutput) {
5389
exists(string type, string input, string output, string path |
90+
// If the summary for 'callable' could not be handled as a flow summary, we need to evaluate
91+
// its inputs and outputs to a set of nodes, so we can generate steps instead.
92+
shouldInduceStepsFromSummary(type, path) and
5493
ModelOutput::resolvedSummaryBase(type, path, base) and
5594
ModelOutput::relevantSummaryModel(type, path, input, output, _, _) and
5695
inputOrOutput = [input, output]
@@ -81,6 +120,7 @@ private API::Node getNodeFromInputOutputPath(API::InvokeNode baseNode, AccessPat
81120

82121
private predicate summaryStep(API::Node pred, API::Node succ, string kind) {
83122
exists(string type, string path, API::InvokeNode base, AccessPath input, AccessPath output |
123+
shouldInduceStepsFromSummary(type, path) and
84124
ModelOutput::relevantSummaryModel(type, path, input, output, kind, _) and
85125
ModelOutput::resolvedSummaryBase(type, path, base) and
86126
pred = getNodeFromInputOutputPath(base, input) and

javascript/ql/test/library-tests/TripleDot/underscore.string.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ function strToStr() {
3939
}
4040

4141
function strToArray() {
42-
sink(s.chop(source("s1"), 3)); // $ MISSING: hasTaintFlow=s1
42+
sink(s.chop(source("s1"), 3)); // $ hasTaintFlow=s1
4343
sink(s.chars(source("s2"))[0]); // $ hasTaintFlow=s2
4444
sink(s.words(source("s3"))[0]); // $ hasTaintFlow=s3
4545
sink(s.lines(source("s7"))[0]); // $ hasTaintFlow=s7
@@ -97,7 +97,7 @@ function multiSource() {
9797

9898
function chaining() {
9999
sink(s(source("s1"))
100-
.slugify().capitalize().decapitalize().clean().cleanDiacritics()
100+
.slugify().capitalize().decapitalize().clean().cleanDiacritics()
101101
.swapCase().escapeHTML().unescapeHTML().wrap().dedent()
102102
.reverse().pred().succ().titleize().camelize().classify()
103103
.underscored().dasherize().humanize().trim().ltrim().rtrim()
@@ -119,8 +119,8 @@ function chaining() {
119119
.q(source("s17")).ljust(10, source("s18"))
120120
.rjust(10, source("s19"))); // $ hasTaintFlow=s16 hasTaintFlow=s17 hasTaintFlow=s18 hasTaintFlow=s19
121121

122-
sink(s(source("s20")).tap(function(value) {
123-
return value + source("s21");
122+
sink(s(source("s20")).tap(function(value) {
123+
return value + source("s21");
124124
}).value()); // $ hasTaintFlow=s20 hasTaintFlow=s21
125125
}
126126

0 commit comments

Comments
 (0)