Skip to content

Commit 07bff64

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request github#3641 from asger-semmle/js/pre-call-graph-steps
Approved by erik-krogh
2 parents d80a033 + c7f74e4 commit 07bff64

File tree

7 files changed

+195
-27
lines changed

7 files changed

+195
-27
lines changed

‎javascript/ql/src/semmle/javascript/Arrays.qll

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import javascript
22
private import semmle.javascript.dataflow.InferredTypes
3+
private import semmle.javascript.dataflow.internal.PreCallGraphStep
34

45
/**
56
* Classes and predicates for modelling TaintTracking steps for arrays.
@@ -222,29 +223,32 @@ private module ArrayDataFlow {
222223
*
223224
* And the second parameter in the callback is the array ifself, so there is a `loadStoreStep` from the array to that second parameter.
224225
*/
225-
private class ArrayIteration extends DataFlow::AdditionalFlowStep, DataFlow::MethodCallNode {
226-
ArrayIteration() {
227-
this.getMethodName() = "map" or
228-
this.getMethodName() = "forEach"
229-
}
230-
226+
private class ArrayIteration extends PreCallGraphStep {
231227
override predicate loadStep(DataFlow::Node obj, DataFlow::Node element, string prop) {
232-
prop = arrayElement() and
233-
obj = this.getReceiver() and
234-
element = getCallback(0).getParameter(0)
228+
exists(DataFlow::MethodCallNode call |
229+
call.getMethodName() = ["map", "forEach"] and
230+
prop = arrayElement() and
231+
obj = call.getReceiver() and
232+
element = call.getCallback(0).getParameter(0)
233+
)
235234
}
236235

237236
override predicate storeStep(DataFlow::Node element, DataFlow::SourceNode obj, string prop) {
238-
this.getMethodName() = "map" and
239-
prop = arrayElement() and
240-
element = this.getCallback(0).getAReturn() and
241-
obj = this
237+
exists(DataFlow::MethodCallNode call |
238+
call.getMethodName() = "map" and
239+
prop = arrayElement() and
240+
element = call.getCallback(0).getAReturn() and
241+
obj = call
242+
)
242243
}
243244

244-
override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
245-
prop = arrayElement() and
246-
pred = this.getReceiver() and
247-
succ = getCallback(0).getParameter(2)
245+
override predicate loadStoreStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
246+
exists(DataFlow::MethodCallNode call |
247+
call.getMethodName() = ["map", "forEach"] and
248+
prop = arrayElement() and
249+
pred = call.getReceiver() and
250+
succ = call.getCallback(0).getParameter(2)
251+
)
248252
}
249253
}
250254

@@ -311,16 +315,13 @@ private module ArrayDataFlow {
311315
/**
312316
* A step for modelling `for of` iteration on arrays.
313317
*/
314-
private class ForOfStep extends DataFlow::AdditionalFlowStep, DataFlow::ValueNode {
315-
ForOfStmt forOf;
316-
DataFlow::Node element;
317-
318-
ForOfStep() { this.asExpr() = forOf.getIterationDomain() }
319-
318+
private class ForOfStep extends PreCallGraphStep {
320319
override predicate loadStep(DataFlow::Node obj, DataFlow::Node e, string prop) {
321-
obj = this and
322-
e = DataFlow::lvalueNode(forOf.getLValue()) and
323-
prop = arrayElement()
320+
exists(ForOfStmt forOf |
321+
obj = forOf.getIterationDomain().flow() and
322+
e = DataFlow::lvalueNode(forOf.getLValue()) and
323+
prop = arrayElement()
324+
)
324325
}
325326
}
326327
}

‎javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ private import internal.CallGraphs
2323
private import internal.FlowSteps as FlowSteps
2424
private import internal.DataFlowNode
2525
private import internal.AnalyzedParameters
26+
private import internal.PreCallGraphStep
2627

2728
module DataFlow {
2829
/**

‎javascript/ql/src/semmle/javascript/dataflow/TypeTracking.qll

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,5 +335,20 @@ abstract class AdditionalTypeTrackingStep extends DataFlow::Node {
335335
/**
336336
* Holds if type-tracking should step from `pred` to `succ`.
337337
*/
338-
abstract predicate step(DataFlow::Node pred, DataFlow::Node succ);
338+
predicate step(DataFlow::Node pred, DataFlow::Node succ) { none() }
339+
340+
/**
341+
* Holds if type-tracking should step from `pred` into the `prop` property of `succ`.
342+
*/
343+
predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) { none() }
344+
345+
/**
346+
* Holds if type-tracking should step from the `prop` property of `pred` to `succ`.
347+
*/
348+
predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
349+
350+
/**
351+
* Holds if type-tracking should step from the `prop` property of `pred` to the same property in `succ`.
352+
*/
353+
predicate loadStoreStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) { none() }
339354
}

‎javascript/ql/src/semmle/javascript/dataflow/internal/PreCallGraphStep.qll

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
/**
2+
* Provides an extension point for contributing flow edges prior
3+
* to call graph construction and type tracking.
4+
*/
5+
6+
private import javascript
7+
8+
private newtype TUnit = MkUnit()
9+
10+
private class Unit extends TUnit {
11+
string toString() { result = "unit" }
12+
}
13+
14+
/**
15+
* Internal extension point for adding flow edges prior to call graph construction
16+
* and type tracking.
17+
*
18+
* Steps added here will be added to both `AdditionalFlowStep` and `AdditionalTypeTrackingStep`.
19+
*
20+
* Contributing steps that rely on type tracking will lead to negative recursion.
21+
*/
22+
class PreCallGraphStep extends Unit {
23+
/**
24+
* Holds if there is a step from `pred` to `succ`.
25+
*/
26+
predicate step(DataFlow::Node pred, DataFlow::Node succ) { none() }
27+
28+
/**
29+
* Holds if there is a step from `pred` into the `prop` property of `succ`.
30+
*/
31+
predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) { none() }
32+
33+
/**
34+
* Holds if there is a step from the `prop` property of `pred` to `succ`.
35+
*/
36+
predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
37+
38+
/**
39+
* Holds if there is a step from the `prop` property of `pred` to the same property in `succ`.
40+
*/
41+
predicate loadStoreStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) { none() }
42+
}
43+
44+
module PreCallGraphStep {
45+
/**
46+
* Holds if there is a step from `pred` to `succ`.
47+
*/
48+
cached
49+
predicate step(DataFlow::Node pred, DataFlow::Node succ) {
50+
any(PreCallGraphStep s).step(pred, succ)
51+
}
52+
53+
/**
54+
* Holds if there is a step from `pred` into the `prop` property of `succ`.
55+
*/
56+
cached
57+
predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
58+
any(PreCallGraphStep s).storeStep(pred, succ, prop)
59+
}
60+
61+
/**
62+
* Holds if there is a step from the `prop` property of `pred` to `succ`.
63+
*/
64+
cached
65+
predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
66+
any(PreCallGraphStep s).loadStep(pred, succ, prop)
67+
}
68+
69+
/**
70+
* Holds if there is a step from the `prop` property of `pred` to the same property in `succ`.
71+
*/
72+
cached
73+
predicate loadStoreStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
74+
any(PreCallGraphStep s).loadStoreStep(pred, succ, prop)
75+
}
76+
}
77+
78+
private class NodeWithPreCallGraphStep extends DataFlow::Node {
79+
NodeWithPreCallGraphStep() {
80+
PreCallGraphStep::step(this, _)
81+
or
82+
PreCallGraphStep::storeStep(this, _, _)
83+
or
84+
PreCallGraphStep::loadStep(this, _, _)
85+
or
86+
PreCallGraphStep::loadStoreStep(this, _, _)
87+
}
88+
}
89+
90+
private class AdditionalFlowStepFromPreCallGraph extends NodeWithPreCallGraphStep,
91+
DataFlow::AdditionalFlowStep {
92+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
93+
pred = this and
94+
PreCallGraphStep::step(this, succ)
95+
}
96+
97+
override predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
98+
pred = this and
99+
PreCallGraphStep::storeStep(this, succ, prop)
100+
}
101+
102+
override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
103+
pred = this and
104+
PreCallGraphStep::loadStep(this, succ, prop)
105+
}
106+
107+
override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
108+
pred = this and
109+
PreCallGraphStep::loadStoreStep(this, succ, prop)
110+
}
111+
}
112+
113+
private class AdditionalTypeTrackingStepFromPreCallGraph extends NodeWithPreCallGraphStep,
114+
DataFlow::AdditionalTypeTrackingStep {
115+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
116+
pred = this and
117+
PreCallGraphStep::step(this, succ)
118+
}
119+
120+
override predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
121+
pred = this and
122+
PreCallGraphStep::storeStep(this, succ, prop)
123+
}
124+
125+
override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
126+
pred = this and
127+
PreCallGraphStep::loadStep(this, succ, prop)
128+
}
129+
130+
override predicate loadStoreStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
131+
pred = this and
132+
PreCallGraphStep::loadStoreStep(this, succ, prop)
133+
}
134+
}

‎javascript/ql/src/semmle/javascript/dataflow/internal/StepSummary.qll

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,15 @@ module StepSummary {
110110
or
111111
basicLoadStep(pred, succ, prop) and
112112
summary = LoadStep(prop)
113+
or
114+
any(AdditionalTypeTrackingStep st).storeStep(pred, succ, prop) and
115+
summary = StoreStep(prop)
116+
or
117+
any(AdditionalTypeTrackingStep st).loadStep(pred, succ, prop) and
118+
summary = LoadStep(prop)
119+
or
120+
any(AdditionalTypeTrackingStep st).loadStoreStep(pred, succ, prop) and
121+
summary = CopyStep(prop)
113122
)
114123
or
115124
any(AdditionalTypeTrackingStep st).step(pred, succ) and

‎javascript/ql/test/library-tests/frameworks/SQL/SqlString.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,5 @@
4141
| spanner.js:19:23:19:32 | "SQL code" |
4242
| spannerImport.js:4:8:4:17 | "SQL code" |
4343
| sqlite.js:7:8:7:45 | "UPDATE ... id = ?" |
44+
| sqliteArray.js:6:12:6:49 | "UPDATE ... id = ?" |
4445
| sqliteImport.js:2:8:2:44 | "UPDATE ... id = ?" |

‎javascript/ql/test/library-tests/frameworks/SQL/sqliteArray.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
var sqlite = require('sqlite3');
2+
3+
let databaseNames = [":memory:", ":foo:"];
4+
let databases = databaseNames.map(name => new sqlite.Database(name));
5+
for (let db of databases) {
6+
db.run("UPDATE tbl SET name = ? WHERE id = ?", "bar", 2);
7+
}

0 commit comments

Comments
 (0)