Skip to content

Commit 2f11f37

Browse files
erik-krogherik-krogh
committed
simplify getALibraryInputParameter by adding more general dataflow for the arguments object
1 parent 11b039c commit 2f11f37

File tree

7 files changed

+47
-43
lines changed

7 files changed

+47
-43
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,14 @@ private module ArrayLibraries {
344344
result = DataFlow::globalVarRef("Array").getAMemberCall("from")
345345
or
346346
result = DataFlow::moduleImport("array-from").getACall()
347+
or
348+
// Array.prototype.slice.call acts the same as Array.from, and is sometimes used with e.g. the arguments object.
349+
result =
350+
DataFlow::globalVarRef("Array")
351+
.getAPropertyRead("prototype")
352+
.getAPropertyRead("slice")
353+
.getAMethodCall("call") and
354+
result.getNumArgument() = 1
347355
}
348356

349357
/**

‎javascript/ql/lib/semmle/javascript/PackageExports.qll

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,34 +18,10 @@ DataFlow::Node getALibraryInputParameter() {
1818
|
1919
result = func.getParameter(any(int arg | arg >= bound))
2020
or
21-
result = getAnArgumentsRead(func.getFunction())
22-
or
2321
result = func.getFunction().getArgumentsVariable().getAnAccess().flow()
2422
)
2523
}
2624

27-
private DataFlow::SourceNode getAnArgumentsRead(Function func) {
28-
exists(DataFlow::PropRead read |
29-
not read.getPropertyName() = "length" and
30-
result = read
31-
|
32-
read.getBase() = func.getArgumentsVariable().getAnAccess().flow()
33-
or
34-
exists(DataFlow::MethodCallNode call |
35-
call =
36-
DataFlow::globalVarRef("Array")
37-
.getAPropertyRead("prototype")
38-
.getAPropertyRead("slice")
39-
.getAMethodCall("call")
40-
or
41-
call = DataFlow::globalVarRef("Array").getAMethodCall("from")
42-
|
43-
call.getArgument(0) = func.getArgumentsVariable().getAnAccess().flow() and
44-
call.flowsTo(read.getBase())
45-
)
46-
)
47-
}
48-
4925
private import NodeModuleResolutionImpl as NodeModule
5026

5127
/**

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1661,9 +1661,7 @@ module DataFlow {
16611661
)
16621662
}
16631663

1664-
/**
1665-
* A step from a reflective parameter node to each parameter.
1666-
*/
1664+
/** A load step from a reflective parameter node to each parameter. */
16671665
private class ReflectiveParamsStep extends PreCallGraphStep {
16681666
override predicate loadStep(DataFlow::Node obj, DataFlow::Node element, string prop) {
16691667
exists(DataFlow::ReflectiveParametersNode params, DataFlow::FunctionNode f, int i |
@@ -1675,6 +1673,17 @@ module DataFlow {
16751673
}
16761674
}
16771675

1676+
/** A taint step from the reflective parameters node to any parameter. */
1677+
private class ReflectiveParamsTaintStep extends TaintTracking::SharedTaintStep {
1678+
override predicate step(DataFlow::Node obj, DataFlow::Node element) {
1679+
exists(DataFlow::ReflectiveParametersNode params, DataFlow::FunctionNode f |
1680+
f.getFunction() = params.getFunction() and
1681+
obj = params and
1682+
element = f.getAParameter()
1683+
)
1684+
}
1685+
}
1686+
16781687
/**
16791688
* Holds if there is a step from `pred` to `succ` through a field accessed through `this` in a class.
16801689
*/

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ typeInferenceMismatch
66
| call-apply.js:25:14:25:21 | source() | call-apply.js:21:1:23:1 | the arguments object of function foo1_sink |
77
| call-apply.js:25:14:25:21 | source() | call-apply.js:27:6:27:32 | reflective call |
88
| call-apply.js:25:14:25:21 | source() | call-apply.js:30:6:30:35 | reflective call |
9+
| call-apply.js:25:14:25:21 | source() | call-apply.js:31:6:31:35 | reflective call |
910
| call-apply.js:25:14:25:21 | source() | call-apply.js:62:3:64:3 | the arguments object of function sinkArguments1 |
1011
| call-apply.js:25:14:25:21 | source() | call-apply.js:65:3:67:3 | the arguments object of function sinkArguments0 |
1112
| call-apply.js:25:14:25:21 | source() | call-apply.js:69:3:72:3 | the arguments object of function fowardArguments |
@@ -54,6 +55,8 @@ typeInferenceMismatch
5455
| call-apply.js:25:14:25:21 | source() | call-apply.js:22:8:22:11 | arg1 |
5556
| call-apply.js:25:14:25:21 | source() | call-apply.js:27:6:27:32 | foo1.ca ... ce, "") |
5657
| call-apply.js:25:14:25:21 | source() | call-apply.js:30:6:30:35 | foo1.ap ... e, ""]) |
58+
| call-apply.js:25:14:25:21 | source() | call-apply.js:31:6:31:35 | foo2.ap ... e, ""]) |
59+
| call-apply.js:25:14:25:21 | source() | call-apply.js:38:6:38:29 | foo1_ap ... e, ""]) |
5760
| call-apply.js:25:14:25:21 | source() | call-apply.js:44:6:44:28 | foo1_ca ... e, ""]) |
5861
| call-apply.js:25:14:25:21 | source() | call-apply.js:45:6:45:28 | foo1_ca ... ource]) |
5962
| call-apply.js:25:14:25:21 | source() | call-apply.js:63:10:63:21 | arguments[1] |

‎javascript/ql/test/query-tests/Security/CWE-400/ReDoS/PolynomialReDoS.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ nodes
2222
| lib/lib.js:27:10:27:19 | id("safe") |
2323
| lib/lib.js:28:13:28:13 | y |
2424
| lib/lib.js:28:13:28:13 | y |
25+
| lib/lib.js:32:32:32:40 | arguments |
26+
| lib/lib.js:32:32:32:40 | arguments |
27+
| lib/lib.js:35:1:37:1 | the arguments object of function usedWithArguments |
28+
| lib/lib.js:35:28:35:31 | name |
29+
| lib/lib.js:36:13:36:16 | name |
30+
| lib/lib.js:36:13:36:16 | name |
2531
| lib/moduleLib/moduleLib.js:1:28:1:31 | name |
2632
| lib/moduleLib/moduleLib.js:1:28:1:31 | name |
2733
| lib/moduleLib/moduleLib.js:2:13:2:16 | name |
@@ -238,6 +244,11 @@ edges
238244
| lib/lib.js:27:6:27:19 | y | lib/lib.js:28:13:28:13 | y |
239245
| lib/lib.js:27:6:27:19 | y | lib/lib.js:28:13:28:13 | y |
240246
| lib/lib.js:27:10:27:19 | id("safe") | lib/lib.js:27:6:27:19 | y |
247+
| lib/lib.js:32:32:32:40 | arguments | lib/lib.js:35:1:37:1 | the arguments object of function usedWithArguments |
248+
| lib/lib.js:32:32:32:40 | arguments | lib/lib.js:35:1:37:1 | the arguments object of function usedWithArguments |
249+
| lib/lib.js:35:1:37:1 | the arguments object of function usedWithArguments | lib/lib.js:35:28:35:31 | name |
250+
| lib/lib.js:35:28:35:31 | name | lib/lib.js:36:13:36:16 | name |
251+
| lib/lib.js:35:28:35:31 | name | lib/lib.js:36:13:36:16 | name |
241252
| lib/moduleLib/moduleLib.js:1:28:1:31 | name | lib/moduleLib/moduleLib.js:2:13:2:16 | name |
242253
| lib/moduleLib/moduleLib.js:1:28:1:31 | name | lib/moduleLib/moduleLib.js:2:13:2:16 | name |
243254
| lib/moduleLib/moduleLib.js:1:28:1:31 | name | lib/moduleLib/moduleLib.js:2:13:2:16 | name |
@@ -428,6 +439,7 @@ edges
428439
| lib/indirect.js:2:5:2:17 | /k*h/.test(x) | lib/indirect.js:1:32:1:32 | x | lib/indirect.js:2:16:2:16 | x | This $@ that depends on $@ may run slow on strings with many repetitions of 'k'. | lib/indirect.js:2:6:2:7 | k* | regular expression | lib/indirect.js:1:32:1:32 | x | library input |
429440
| lib/lib.js:4:2:4:18 | regexp.test(name) | lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'a'. | lib/lib.js:1:15:1:16 | a* | regular expression | lib/lib.js:3:28:3:31 | name | library input |
430441
| lib/lib.js:8:2:8:17 | /f*g/.test(name) | lib/lib.js:7:19:7:22 | name | lib/lib.js:8:13:8:16 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'f'. | lib/lib.js:8:3:8:4 | f* | regular expression | lib/lib.js:7:19:7:22 | name | library input |
442+
| lib/lib.js:36:2:36:17 | /f*g/.test(name) | lib/lib.js:32:32:32:40 | arguments | lib/lib.js:36:13:36:16 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'f'. | lib/lib.js:36:3:36:4 | f* | regular expression | lib/lib.js:32:32:32:40 | arguments | library input |
431443
| lib/moduleLib/moduleLib.js:2:2:2:17 | /a*b/.test(name) | lib/moduleLib/moduleLib.js:1:28:1:31 | name | lib/moduleLib/moduleLib.js:2:13:2:16 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'a'. | lib/moduleLib/moduleLib.js:2:3:2:4 | a* | regular expression | lib/moduleLib/moduleLib.js:1:28:1:31 | name | library input |
432444
| lib/otherLib/js/src/index.js:2:2:2:17 | /a*b/.test(name) | lib/otherLib/js/src/index.js:1:28:1:31 | name | lib/otherLib/js/src/index.js:2:13:2:16 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'a'. | lib/otherLib/js/src/index.js:2:3:2:4 | a* | regular expression | lib/otherLib/js/src/index.js:1:28:1:31 | name | library input |
433445
| lib/snapdragon.js:7:15:7:32 | this.match(/aa*$/) | lib/snapdragon.js:3:34:3:38 | input | lib/snapdragon.js:7:15:7:18 | this | This $@ that depends on $@ may run slow on strings starting with 'a' and with many repetitions of 'a'. | lib/snapdragon.js:7:28:7:29 | a* | regular expression | lib/snapdragon.js:3:34:3:38 | input | library input |

‎javascript/ql/test/query-tests/Security/CWE-400/ReDoS/lib/lib.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ module.exports.useArguments = function () {
3333
}
3434

3535
function usedWithArguments(name) {
36-
/f*g/.test(name); // NOT OK - bit not yet recognized [INCONSITENCY]
36+
/f*g/.test(name); // NOT OK
3737
}
3838

3939
module.exports.snapdragon = require("./snapdragon")

‎javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingAssignment/PrototypePollutingAssignment.expected

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ nodes
2929
| lib.js:20:14:20:22 | arguments |
3030
| lib.js:20:14:20:22 | arguments |
3131
| lib.js:20:14:20:25 | arguments[1] |
32-
| lib.js:20:14:20:25 | arguments[1] |
3332
| lib.js:22:3:22:14 | obj[path[0]] |
3433
| lib.js:22:3:22:14 | obj[path[0]] |
3534
| lib.js:22:7:22:10 | path |
@@ -40,8 +39,12 @@ nodes
4039
| lib.js:26:10:26:21 | obj[path[0]] |
4140
| lib.js:26:14:26:17 | path |
4241
| lib.js:26:14:26:20 | path[0] |
42+
| lib.js:30:9:30:52 | args |
43+
| lib.js:30:16:30:52 | Array.p ... uments) |
44+
| lib.js:30:43:30:51 | arguments |
45+
| lib.js:30:43:30:51 | arguments |
4346
| lib.js:32:7:32:20 | path |
44-
| lib.js:32:14:32:20 | args[1] |
47+
| lib.js:32:14:32:17 | args |
4548
| lib.js:32:14:32:20 | args[1] |
4649
| lib.js:34:3:34:14 | obj[path[0]] |
4750
| lib.js:34:3:34:14 | obj[path[0]] |
@@ -54,7 +57,6 @@ nodes
5457
| lib.js:40:7:40:20 | path |
5558
| lib.js:40:14:40:17 | args |
5659
| lib.js:40:14:40:20 | args[1] |
57-
| lib.js:40:14:40:20 | args[1] |
5860
| lib.js:42:3:42:14 | obj[path[0]] |
5961
| lib.js:42:3:42:14 | obj[path[0]] |
6062
| lib.js:42:7:42:10 | path |
@@ -81,7 +83,6 @@ nodes
8183
| lib.js:83:14:83:22 | arguments |
8284
| lib.js:83:14:83:22 | arguments |
8385
| lib.js:83:14:83:25 | arguments[1] |
84-
| lib.js:83:14:83:25 | arguments[1] |
8586
| lib.js:86:7:86:26 | proto |
8687
| lib.js:86:15:86:26 | obj[path[0]] |
8788
| lib.js:86:19:86:22 | path |
@@ -101,7 +102,6 @@ nodes
101102
| lib.js:104:13:104:21 | arguments |
102103
| lib.js:104:13:104:21 | arguments |
103104
| lib.js:104:13:104:24 | arguments[1] |
104-
| lib.js:104:13:104:24 | arguments[1] |
105105
| lib.js:108:3:108:10 | obj[one] |
106106
| lib.js:108:3:108:10 | obj[one] |
107107
| lib.js:108:7:108:9 | one |
@@ -197,7 +197,6 @@ edges
197197
| lib.js:20:14:20:22 | arguments | lib.js:20:14:20:25 | arguments[1] |
198198
| lib.js:20:14:20:22 | arguments | lib.js:20:14:20:25 | arguments[1] |
199199
| lib.js:20:14:20:25 | arguments[1] | lib.js:20:7:20:25 | path |
200-
| lib.js:20:14:20:25 | arguments[1] | lib.js:20:7:20:25 | path |
201200
| lib.js:22:7:22:10 | path | lib.js:22:7:22:13 | path[0] |
202201
| lib.js:22:7:22:13 | path[0] | lib.js:22:3:22:14 | obj[path[0]] |
203202
| lib.js:22:7:22:13 | path[0] | lib.js:22:3:22:14 | obj[path[0]] |
@@ -206,8 +205,12 @@ edges
206205
| lib.js:26:14:26:17 | path | lib.js:26:14:26:20 | path[0] |
207206
| lib.js:26:14:26:20 | path[0] | lib.js:26:10:26:21 | obj[path[0]] |
208207
| lib.js:26:14:26:20 | path[0] | lib.js:26:10:26:21 | obj[path[0]] |
208+
| lib.js:30:9:30:52 | args | lib.js:32:14:32:17 | args |
209+
| lib.js:30:16:30:52 | Array.p ... uments) | lib.js:30:9:30:52 | args |
210+
| lib.js:30:43:30:51 | arguments | lib.js:30:16:30:52 | Array.p ... uments) |
211+
| lib.js:30:43:30:51 | arguments | lib.js:30:16:30:52 | Array.p ... uments) |
209212
| lib.js:32:7:32:20 | path | lib.js:34:7:34:10 | path |
210-
| lib.js:32:14:32:20 | args[1] | lib.js:32:7:32:20 | path |
213+
| lib.js:32:14:32:17 | args | lib.js:32:14:32:20 | args[1] |
211214
| lib.js:32:14:32:20 | args[1] | lib.js:32:7:32:20 | path |
212215
| lib.js:34:7:34:10 | path | lib.js:34:7:34:13 | path[0] |
213216
| lib.js:34:7:34:13 | path[0] | lib.js:34:3:34:14 | obj[path[0]] |
@@ -219,7 +222,6 @@ edges
219222
| lib.js:40:7:40:20 | path | lib.js:42:7:42:10 | path |
220223
| lib.js:40:14:40:17 | args | lib.js:40:14:40:20 | args[1] |
221224
| lib.js:40:14:40:20 | args[1] | lib.js:40:7:40:20 | path |
222-
| lib.js:40:14:40:20 | args[1] | lib.js:40:7:40:20 | path |
223225
| lib.js:42:7:42:10 | path | lib.js:42:7:42:13 | path[0] |
224226
| lib.js:42:7:42:13 | path[0] | lib.js:42:3:42:14 | obj[path[0]] |
225227
| lib.js:42:7:42:13 | path[0] | lib.js:42:3:42:14 | obj[path[0]] |
@@ -243,7 +245,6 @@ edges
243245
| lib.js:83:14:83:22 | arguments | lib.js:83:14:83:25 | arguments[1] |
244246
| lib.js:83:14:83:22 | arguments | lib.js:83:14:83:25 | arguments[1] |
245247
| lib.js:83:14:83:25 | arguments[1] | lib.js:83:7:83:25 | path |
246-
| lib.js:83:14:83:25 | arguments[1] | lib.js:83:7:83:25 | path |
247248
| lib.js:86:7:86:26 | proto | lib.js:87:10:87:14 | proto |
248249
| lib.js:86:7:86:26 | proto | lib.js:87:10:87:14 | proto |
249250
| lib.js:86:15:86:26 | obj[path[0]] | lib.js:86:7:86:26 | proto |
@@ -261,7 +262,6 @@ edges
261262
| lib.js:104:13:104:21 | arguments | lib.js:104:13:104:24 | arguments[1] |
262263
| lib.js:104:13:104:21 | arguments | lib.js:104:13:104:24 | arguments[1] |
263264
| lib.js:104:13:104:24 | arguments[1] | lib.js:104:7:104:24 | one |
264-
| lib.js:104:13:104:24 | arguments[1] | lib.js:104:7:104:24 | one |
265265
| lib.js:108:7:108:9 | one | lib.js:108:3:108:10 | obj[one] |
266266
| lib.js:108:7:108:9 | one | lib.js:108:3:108:10 | obj[one] |
267267
| lib.js:118:29:118:32 | path | lib.js:119:17:119:20 | path |
@@ -322,16 +322,12 @@ edges
322322
| lib.js:6:7:6:9 | obj | lib.js:1:43:1:46 | path | lib.js:6:7:6:9 | obj | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:1:43:1:46 | path | library input |
323323
| lib.js:15:3:15:14 | obj[path[0]] | lib.js:14:38:14:41 | path | lib.js:15:3:15:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:14:38:14:41 | path | library input |
324324
| lib.js:22:3:22:14 | obj[path[0]] | lib.js:20:14:20:22 | arguments | lib.js:22:3:22:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:20:14:20:22 | arguments | library input |
325-
| lib.js:22:3:22:14 | obj[path[0]] | lib.js:20:14:20:25 | arguments[1] | lib.js:22:3:22:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:20:14:20:25 | arguments[1] | library input |
326325
| lib.js:26:10:26:21 | obj[path[0]] | lib.js:25:44:25:47 | path | lib.js:26:10:26:21 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:25:44:25:47 | path | library input |
327-
| lib.js:34:3:34:14 | obj[path[0]] | lib.js:32:14:32:20 | args[1] | lib.js:34:3:34:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:32:14:32:20 | args[1] | library input |
326+
| lib.js:34:3:34:14 | obj[path[0]] | lib.js:30:43:30:51 | arguments | lib.js:34:3:34:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:30:43:30:51 | arguments | library input |
328327
| lib.js:42:3:42:14 | obj[path[0]] | lib.js:38:27:38:35 | arguments | lib.js:42:3:42:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:38:27:38:35 | arguments | library input |
329-
| lib.js:42:3:42:14 | obj[path[0]] | lib.js:40:14:40:20 | args[1] | lib.js:42:3:42:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:40:14:40:20 | args[1] | library input |
330328
| lib.js:70:13:70:24 | obj[path[0]] | lib.js:59:18:59:18 | s | lib.js:70:13:70:24 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:59:18:59:18 | s | library input |
331329
| lib.js:87:10:87:14 | proto | lib.js:83:14:83:22 | arguments | lib.js:87:10:87:14 | proto | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:83:14:83:22 | arguments | library input |
332-
| lib.js:87:10:87:14 | proto | lib.js:83:14:83:25 | arguments[1] | lib.js:87:10:87:14 | proto | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:83:14:83:25 | arguments[1] | library input |
333330
| lib.js:108:3:108:10 | obj[one] | lib.js:104:13:104:21 | arguments | lib.js:108:3:108:10 | obj[one] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:104:13:104:21 | arguments | library input |
334-
| lib.js:108:3:108:10 | obj[one] | lib.js:104:13:104:24 | arguments[1] | lib.js:108:3:108:10 | obj[one] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:104:13:104:24 | arguments[1] | library input |
335331
| lib.js:119:13:119:24 | obj[path[0]] | lib.js:118:29:118:32 | path | lib.js:119:13:119:24 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | lib.js:118:29:118:32 | path | library input |
336332
| sublib/sub.js:2:3:2:14 | obj[path[0]] | sublib/sub.js:1:37:1:40 | path | sublib/sub.js:2:3:2:14 | obj[path[0]] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | sublib/sub.js:1:37:1:40 | path | library input |
337333
| tst.js:8:5:8:17 | object[taint] | tst.js:5:24:5:37 | req.query.data | tst.js:8:5:8:17 | object[taint] | This assignment may alter Object.prototype if a malicious '__proto__' string is injected from $@. | tst.js:5:24:5:37 | req.query.data | user controlled input |

0 commit comments

Comments
 (0)