Skip to content

Commit 9d3a9d7

Browse files
author
Max Schaefer
committed
JavaScript: Add basic support for reasoning about reflective parameter accesses.
Currently, only `arguments[c]` for a constant value `c` is supported. This allows us to detect the prototype-pollution vulnerabilities in (old versions of) `extend`, `jquery`, and `node.extend`.
1 parent a39e8b4 commit 9d3a9d7

File tree

3 files changed

+40
-5
lines changed

3 files changed

+40
-5
lines changed

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

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,14 @@ private module CachedSteps {
151151
) {
152152
calls(invk, f) and
153153
(
154-
exists(int i, Parameter p |
155-
f.getParameter(i) = p and
156-
not p.isRestParameter() and
157-
arg = invk.getArgument(i) and
158-
parm = DataFlow::parameterNode(p)
154+
exists(int i | arg = invk.getArgument(i) |
155+
exists(Parameter p |
156+
f.getParameter(i) = p and
157+
not p.isRestParameter() and
158+
parm = DataFlow::parameterNode(p)
159+
)
160+
or
161+
parm = reflectiveParameterAccess(f, i)
159162
)
160163
or
161164
arg = invk.(DataFlow::CallNode).getReceiver() and
@@ -185,6 +188,22 @@ private module CachedSteps {
185188
)
186189
}
187190

191+
/**
192+
* Gets a data-flow node inside `f` that refers to the `arguments` object of `f`.
193+
*/
194+
private DataFlow::Node argumentsAccess(Function f) {
195+
result.getContainer().getEnclosingContainer*() = f and
196+
result.analyze().getAValue().(AbstractArguments).getFunction() = f
197+
}
198+
199+
/**
200+
* Gets a data-flow node that refers to the `i`th parameter of `f` through its `arguments`
201+
* object.
202+
*/
203+
private DataFlow::SourceNode reflectiveParameterAccess(Function f, int i) {
204+
result.(DataFlow::PropRead).accesses(argumentsAccess(f), any(string p | i = p.toInt()))
205+
}
206+
188207
/**
189208
* Holds if there is a flow step from `pred` to `succ` through parameter passing
190209
* to a function call.

javascript/ql/test/library-tests/DataFlow/argumentPassing.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
| arguments.js:11:5:11:14 | f(1, 2, 3) | arguments.js:11:7:11:7 | 1 | arguments.js:2:5:10:5 | functio ... ;\\n } | arguments.js:2:16:2:16 | x |
2+
| arguments.js:11:5:11:14 | f(1, 2, 3) | arguments.js:11:7:11:7 | 1 | arguments.js:2:5:10:5 | functio ... ;\\n } | arguments.js:4:28:4:39 | arguments[0] |
3+
| arguments.js:11:5:11:14 | f(1, 2, 3) | arguments.js:11:10:11:10 | 2 | arguments.js:2:5:10:5 | functio ... ;\\n } | arguments.js:5:25:5:36 | arguments[1] |
4+
| arguments.js:11:5:11:14 | f(1, 2, 3) | arguments.js:11:13:11:13 | 3 | arguments.js:2:5:10:5 | functio ... ;\\n } | arguments.js:7:24:7:30 | args[2] |
15
| sources.js:3:1:5:6 | (functi ... \\n})(23) | sources.js:5:4:5:5 | 23 | sources.js:3:2:5:1 | functio ... x+19;\\n} | sources.js:3:11:3:11 | x |
26
| tst.js:16:1:20:9 | (functi ... ("arg") | tst.js:20:4:20:8 | "arg" | tst.js:16:2:20:1 | functio ... n "";\\n} | tst.js:16:13:16:13 | a |
37
| tst.js:35:1:35:7 | g(true) | tst.js:35:3:35:6 | true | tst.js:32:1:34:1 | functio ... ables\\n} | tst.js:32:12:32:12 | b |
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
(function() {
2+
function f(x) {
3+
let firstArg = x;
4+
let alsoFirstArg = arguments[0];
5+
let secondArg = arguments[1];
6+
let args = arguments;
7+
let thirdArg = args[2];
8+
arguments = {};
9+
let notFirstArg = arguments[0];
10+
}
11+
f(1, 2, 3);
12+
})();

0 commit comments

Comments
 (0)