Skip to content

Commit 3ca7d6b

Browse files
committed
JavaScript: address comments
1 parent 269bbc9 commit 3ca7d6b

File tree

4 files changed

+66
-3
lines changed

4 files changed

+66
-3
lines changed

javascript/ql/src/semmle/javascript/dataflow/Configuration.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ private class LibraryPartialCall extends AdditionalPartialInvokeNode {
337337
override predicate isPartialArgument(DataFlow::Node callback, DataFlow::Node argument, int index) {
338338
callback = getArgument(0) and
339339
exists (DataFlow::ArrayLiteralNode array |
340-
array = getArgument(1) and
340+
array.flowsTo(getArgument(1)) and
341341
argument = array.getElement(index))
342342
}
343343
}

javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ predicate calls(DataFlow::InvokeNode invk, Function f) {
3232
*
3333
* This only holds for explicitly modeled partial calls.
3434
*/
35-
predicate partiallyCalls(DataFlow::AdditionalPartialInvokeNode invk, DataFlow::Node callback, Function f) {
35+
private predicate partiallyCalls(DataFlow::AdditionalPartialInvokeNode invk, DataFlow::AnalyzedNode callback, Function f) {
3636
invk.isPartialArgument(callback, _, _) and
37-
exists (AbstractFunction callee | callee = callback.analyze().getAValue() |
37+
exists (AbstractFunction callee | callee = callback.getAValue() |
3838
if invk.isIndefinite("global") then
3939
(f = callee.getFunction() and f.getFile() = invk.getFile())
4040
else

javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:17:14:17:14 | x |
2+
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:20:14:20:14 | y |
3+
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:30:14:30:20 | x.value |
4+
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:41:10:41:18 | id(taint) |
5+
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:51:14:51:14 | x |
16
| tst.js:2:13:2:20 | source() | tst.js:4:10:4:10 | x |
27
| tst.js:2:13:2:20 | source() | tst.js:5:10:5:22 | "/" + x + "!" |
38
| tst.js:2:13:2:20 | source() | tst.js:14:10:14:17 | x.sort() |
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
let R = require('ramda');
2+
3+
function test() {
4+
let taint = source();
5+
6+
function safe1(x, y) {
7+
sink(x); // OK - x is not tainted
8+
}
9+
function safe2(x, y) {
10+
sink(y); // OK - y is not tainted
11+
}
12+
13+
safe1.bind(null, "hello", taint)();
14+
safe2.bind(null, taint, "hello")();
15+
16+
function unsafe1(x, y) {
17+
sink(x); // NOT OK - x is tainted
18+
}
19+
function unsafe2(x ,y) {
20+
sink(y); // NOT OK - y is tainted
21+
}
22+
23+
unsafe1.bind(null, taint, "hello")();
24+
unsafe2.bind(null, "hello", taint)();
25+
26+
function safeprop(x) {
27+
sink(x.value); // OK - property `value` is not tainted
28+
}
29+
function unsafeprop(x) {
30+
sink(x.value); // NOT OK - property `value` is tainted
31+
}
32+
33+
safeprop.bind(null, {value: "hello", somethingElse: taint})();
34+
unsafeprop.bind(null, {value: taint, somethingElse: "hello"})();
35+
36+
function id(x) {
37+
return x;
38+
}
39+
40+
sink(id("hello")); // OK
41+
sink(id(taint)); // NOT OK
42+
43+
let taintGetter = id.bind(null, taint);
44+
sink(taintGetter); // OK - this is a function object
45+
sink(taintGetter()); // NOT OK - but not currently detected
46+
47+
function safearray(x) {
48+
sink(x); // OK
49+
}
50+
function unsafearray(x) {
51+
sink(x); // NOT OK
52+
}
53+
54+
let xs = ["hello"];
55+
let ys = [taint];
56+
R.partial(safearray, xs)();
57+
R.partial(unsafearray, ys)();
58+
}

0 commit comments

Comments
 (0)