hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3564 from max-schaefer/js/reflective-argument-access

Browse Source 
Approved by asgerf
This commit is contained in:
semmle-qlci
2020-05-26 12:09:13 +01:00
committed by GitHub
parent 4b0354c4bc 5b0a3b9673
commit be5b343a0c
11 changed files with 160 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
change-notes/1.25/analysis-javascript.md
View File

@@ -3,6 +3,7 @@
## General improvements
* Support for the following frameworks and libraries has been improved:
- [Promise](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise)
- [bluebird](http://bluebirdjs.com/)
- [express](https://www.npmjs.com/package/express)
- [fastify](https://www.npmjs.com/package/fastify)
@@ -14,12 +15,11 @@
- [mssql](https://www.npmjs.com/package/mssql)
- [mysql](https://www.npmjs.com/package/mysql)
- [pg](https://www.npmjs.com/package/pg)
- [Promise](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise)
- [sequelize](https://www.npmjs.com/package/sequelize)
- [spanner](https://www.npmjs.com/package/spanner)
- [sqlite](https://www.npmjs.com/package/sqlite)
- [ssh2](https://www.npmjs.com/package/ssh2)
- [ssh2-streams](https://www.npmjs.com/package/ssh2-streams)
- [ssh2](https://www.npmjs.com/package/ssh2)
* TypeScript 3.9 is now supported.
@@ -36,41 +36,42 @@
| **Query** | **Expected impact** | **Change** |
|--------------------------------|------------------------------|---------------------------------------------------------------------------|
| Misspelled variable name (`js/misspelled-variable-name`) | Message changed | The message for this query now correctly identifies the misspelled variable in additional cases. |
| Uncontrolled data used in path expression (`js/path-injection`) | More results | This query now recognizes additional file system calls. |
| Uncontrolled command line (`js/command-line-injection`) | More results | This query now recognizes additional command execution calls. |
| Client-side URL redirect (`js/client-side-unvalidated-url-redirection`) | Less results | This query now recognizes additional safe patterns of doing URL redirects. |
| Client-side cross-site scripting (`js/xss`) | Less results | This query now recognizes additional safe strings based on URLs. |
| Incomplete URL scheme check (`js/incomplete-url-scheme-check`) | More results | This query now recognizes additional url scheme checks. |
| Prototype pollution in utility function (`js/prototype-pollution-utility`) | More results | This query now recognizes additional utility functions as vulnerable to prototype polution. |
| Expression has no effect (`js/useless-expression`) | Less results | This query no longer flags an expression when that expression is the only content of the containing file. |
| Unknown directive (`js/unknown-directive`) | Less results | This query no longer flags directives generated by the Babel compiler. |
| Client-side URL redirect (`js/client-side-unvalidated-url-redirection`) | Fewer results | This query now recognizes additional safe patterns of doing URL redirects. |
| Client-side cross-site scripting (`js/xss`) | Fewer results | This query now recognizes additional safe strings based on URLs. |
| Code injection (`js/code-injection`) | More results | More potential vulnerabilities involving NoSQL code operators are now recognized. |
| Expression has no effect (`js/useless-expression`) | Fewer results | This query no longer flags an expression when that expression is the only content of the containing file. |
| Incomplete URL scheme check (`js/incomplete-url-scheme-check`) | More results | This query now recognizes additional url scheme checks. |
| Misspelled variable name (`js/misspelled-variable-name`) | Message changed | The message for this query now correctly identifies the misspelled variable in additional cases. |
| Prototype pollution in utility function (`js/prototype-pollution-utility`) | More results | This query now recognizes additional utility functions as vulnerable to prototype polution. |
| Prototype pollution in utility function (`js/prototype-pollution-utility`) | More results | This query now recognizes more coding patterns that are vulnerable to prototype pollution. |
| Uncontrolled command line (`js/command-line-injection`) | More results | This query now recognizes additional command execution calls. |
| Uncontrolled data used in path expression (`js/path-injection`) | More results | This query now recognizes additional file system calls. |
| Unknown directive (`js/unknown-directive`) | Fewer results | This query no longer flags directives generated by the Babel compiler. |
| Unused property (`js/unused-property`) | Fewer results | This query no longer flags properties of objects that are operands of `yield` expressions. |
| Zip Slip (`js/zipslip`) | More results | This query now recognizes additional vulnerabilities. |
| Unused property (`js/unused-property`) | Less results | This query no longer flags properties of objects that are operands of `yield` expressions. |
The following low-precision queries are no longer run by default on LGTM (their results already were not displayed):
- `js/angular/dead-event-listener`
- `js/angular/unused-dependency`
- `js/conflicting-html-attribute`
- `js/useless-assignment-to-global`
- `js/too-many-parameters`
- `js/unused-property`
- `js/bitwise-sign-check`
- `js/comparison-of-identical-expressions`
- `js/misspelled-identifier`
- `js/jsdoc/malformed-param-tag`
- `js/jsdoc/unknown-parameter`
- `js/jsdoc/missing-parameter`
- `js/omitted-array-element`
- `js/conflicting-html-attribute`
- `js/ignored-setter-parameter`
- `js/jsdoc/malformed-param-tag`
- `js/jsdoc/missing-parameter`
- `js/jsdoc/unknown-parameter`
- `js/json-in-javascript-file`
- `js/misspelled-identifier`
- `js/nested-loops-with-same-variable`
- `js/node/cyclic-import`
- `js/node/unused-npm-dependency`
- `js/single-run-loop`
- `js/nested-loops-with-same-variable`
- `js/omitted-array-element`
- `js/return-outside-function`
- `js/single-run-loop`
- `js/too-many-parameters`
- `js/unused-property`
- `js/useless-assignment-to-global`
## Changes to libraries
@@ -80,3 +81,4 @@ The following low-precision queries are no longer run by default on LGTM (their
- `Parameter.flow()` now gets the correct data flow node for a parameter. Previously this had a result, but the node was disconnected from the data flow graph.
- `ParameterNode.asExpr()` and `.getAstNode()` now gets the parameter's AST node, whereas previously it had no result.
- `Expr.flow()` now has a more meaningful result for destructuring patterns. Previously this node was disconnected from the data flow graph. Now it represents the values being destructured by the pattern.
* The global data-flow and taint-tracking libraries now model indirect parameter accesses through the `arguments` object in some cases, which may lead to additional results from some of the security queries, particularly "Prototype pollution in utility function".

29
javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
View File

@@ -151,11 +151,14 @@ private module CachedSteps {
) {
calls(invk, f) and
(
exists(int i, Parameter p |
f.getParameter(i) = p and
not p.isRestParameter() and
arg = invk.getArgument(i) and
parm = DataFlow::parameterNode(p)
exists(int i | arg = invk.getArgument(i) |
exists(Parameter p |
f.getParameter(i) = p and
not p.isRestParameter() and
parm = DataFlow::parameterNode(p)
)
or
parm = reflectiveParameterAccess(f, i)
)
or
arg = invk.(DataFlow::CallNode).getReceiver() and
@@ -185,6 +188,22 @@ private module CachedSteps {
)
}
/**
* Gets a data-flow node inside `f` that refers to the `arguments` object of `f`.
*/
private DataFlow::Node argumentsAccess(Function f) {
result.getContainer().getEnclosingContainer*() = f and
result.analyze().getAValue().(AbstractArguments).getFunction() = f
}
/**
* Gets a data-flow node that refers to the `i`th parameter of `f` through its `arguments`
* object.
*/
private DataFlow::SourceNode reflectiveParameterAccess(Function f, int i) {
result.(DataFlow::PropRead).accesses(argumentsAccess(f), any(string p | i = p.toInt()))
}
/**
* Holds if there is a flow step from `pred` to `succ` through parameter passing
* to a function call.

10
javascript/ql/test/library-tests/DataFlow/argumentPassing.expected Normal file
View File

@@ -0,0 +1,10 @@
| 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 |
| 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] |
| 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] |
| 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] |
| 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 |
| 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 |
| 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 |
| tst.js:44:1:44:5 | o.m() | tst.js:44:1:44:1 | o | tst.js:39:4:41:3 | () {\\n this;\\n } | tst.js:39:4:39:3 | this |
| tst.js:87:1:96:2 | (functi ... r: 0\\n}) | tst.js:92:4:96:1 | {\\n p: ... r: 0\\n} | tst.js:87:2:92:1 | functio ... + z;\\n} | tst.js:87:11:87:24 | { p: x, ...o } |
| tst.js:98:1:103:17 | (functi ... 3, 0 ]) | tst.js:103:4:103:16 | [ 19, 23, 0 ] | tst.js:98:2:103:1 | functio ... + z;\\n} | tst.js:98:11:98:24 | [ x, ...rest ] |

6
javascript/ql/test/library-tests/DataFlow/argumentPassing.ql Normal file
View File

@@ -0,0 +1,6 @@
import javascript
import semmle.javascript.dataflow.internal.FlowSteps as FlowSteps
from DataFlow::Node invk, DataFlow::Node arg, Function f, DataFlow::SourceNode parm
where FlowSteps::argumentPassing(invk, arg, f, parm)
select invk, arg, f, parm

12
javascript/ql/test/library-tests/DataFlow/arguments.js Normal file
View File

@@ -0,0 +1,12 @@
(function() {
function f(x) {
let firstArg = x;
let alsoFirstArg = arguments[0];
let secondArg = arguments[1];
let args = arguments;
let thirdArg = args[2];
arguments = {};
let notFirstArg = arguments[0];
}
f(1, 2, 3);
})();

39
javascript/ql/test/library-tests/DataFlow/enclosingExpr.expected
View File

@@ -1,3 +1,42 @@
| arguments.js:1:1:12:2 | (functi ... 3);\\n}) | arguments.js:1:1:12:2 | (functi ... 3);\\n}) |
| arguments.js:1:1:12:4 | (functi ... );\\n})() | arguments.js:1:1:12:4 | (functi ... );\\n})() |
| arguments.js:1:2:12:1 | functio ... , 3);\\n} | arguments.js:1:2:12:1 | functio ... , 3);\\n} |
| arguments.js:2:14:2:14 | f | arguments.js:2:14:2:14 | f |
| arguments.js:2:16:2:16 | x | arguments.js:2:16:2:16 | x |
| arguments.js:3:13:3:20 | firstArg | arguments.js:3:13:3:20 | firstArg |
| arguments.js:3:13:3:24 | firstArg = x | arguments.js:3:13:3:24 | firstArg = x |
| arguments.js:3:24:3:24 | x | arguments.js:3:24:3:24 | x |
| arguments.js:4:13:4:24 | alsoFirstArg | arguments.js:4:13:4:24 | alsoFirstArg |
| arguments.js:4:13:4:39 | alsoFir ... ents[0] | arguments.js:4:13:4:39 | alsoFir ... ents[0] |
| arguments.js:4:28:4:36 | arguments | arguments.js:4:28:4:36 | arguments |
| arguments.js:4:28:4:39 | arguments[0] | arguments.js:4:28:4:39 | arguments[0] |
| arguments.js:4:38:4:38 | 0 | arguments.js:4:38:4:38 | 0 |
| arguments.js:5:13:5:21 | secondArg | arguments.js:5:13:5:21 | secondArg |
| arguments.js:5:13:5:36 | secondA ... ents[1] | arguments.js:5:13:5:36 | secondA ... ents[1] |
| arguments.js:5:25:5:33 | arguments | arguments.js:5:25:5:33 | arguments |
| arguments.js:5:25:5:36 | arguments[1] | arguments.js:5:25:5:36 | arguments[1] |
| arguments.js:5:35:5:35 | 1 | arguments.js:5:35:5:35 | 1 |
| arguments.js:6:13:6:16 | args | arguments.js:6:13:6:16 | args |
| arguments.js:6:13:6:28 | args = arguments | arguments.js:6:13:6:28 | args = arguments |
| arguments.js:6:20:6:28 | arguments | arguments.js:6:20:6:28 | arguments |
| arguments.js:7:13:7:20 | thirdArg | arguments.js:7:13:7:20 | thirdArg |
| arguments.js:7:13:7:30 | thirdArg = args[2] | arguments.js:7:13:7:30 | thirdArg = args[2] |
| arguments.js:7:24:7:27 | args | arguments.js:7:24:7:27 | args |
| arguments.js:7:24:7:30 | args[2] | arguments.js:7:24:7:30 | args[2] |
| arguments.js:7:29:7:29 | 2 | arguments.js:7:29:7:29 | 2 |
| arguments.js:8:9:8:17 | arguments | arguments.js:8:9:8:17 | arguments |
| arguments.js:8:9:8:22 | arguments = {} | arguments.js:8:9:8:22 | arguments = {} |
| arguments.js:8:21:8:22 | {} | arguments.js:8:21:8:22 | {} |
| arguments.js:9:13:9:23 | notFirstArg | arguments.js:9:13:9:23 | notFirstArg |
| arguments.js:9:13:9:38 | notFirs ... ents[0] | arguments.js:9:13:9:38 | notFirs ... ents[0] |
| arguments.js:9:27:9:35 | arguments | arguments.js:9:27:9:35 | arguments |
| arguments.js:9:27:9:38 | arguments[0] | arguments.js:9:27:9:38 | arguments[0] |
| arguments.js:9:37:9:37 | 0 | arguments.js:9:37:9:37 | 0 |
| arguments.js:11:5:11:5 | f | arguments.js:11:5:11:5 | f |
| arguments.js:11:5:11:14 | f(1, 2, 3) | arguments.js:11:5:11:14 | f(1, 2, 3) |
| arguments.js:11:7:11:7 | 1 | arguments.js:11:7:11:7 | 1 |
| arguments.js:11:10:11:10 | 2 | arguments.js:11:10:11:10 | 2 |
| arguments.js:11:13:11:13 | 3 | arguments.js:11:13:11:13 | 3 |
| eval.js:1:10:1:10 | k | eval.js:1:10:1:10 | k |
| eval.js:2:7:2:7 | x | eval.js:2:7:2:7 | x |
| eval.js:2:7:2:12 | x = 42 | eval.js:2:7:2:12 | x = 42 |

13
javascript/ql/test/library-tests/DataFlow/flowStep.expected
View File

@@ -1,3 +1,16 @@
| arguments.js:1:2:12:1 | functio ... , 3);\\n} | arguments.js:1:1:12:2 | (functi ... 3);\\n}) |
| arguments.js:2:5:2:5 | arguments | arguments.js:4:28:4:36 | arguments |
| arguments.js:2:5:2:5 | arguments | arguments.js:5:25:5:33 | arguments |
| arguments.js:2:5:2:5 | arguments | arguments.js:6:20:6:28 | arguments |
| arguments.js:2:5:10:5 | functio ... ;\\n } | arguments.js:2:14:2:14 | f |
| arguments.js:2:14:2:14 | f | arguments.js:11:5:11:5 | f |
| arguments.js:2:16:2:16 | x | arguments.js:2:16:2:16 | x |
| arguments.js:2:16:2:16 | x | arguments.js:3:24:3:24 | x |
| arguments.js:6:13:6:28 | args | arguments.js:7:24:7:27 | args |
| arguments.js:6:20:6:28 | arguments | arguments.js:6:13:6:28 | args |
| arguments.js:8:9:8:22 | arguments | arguments.js:9:27:9:35 | arguments |
| arguments.js:8:21:8:22 | {} | arguments.js:8:9:8:22 | arguments |
| arguments.js:8:21:8:22 | {} | arguments.js:8:9:8:22 | arguments = {} |
| eval.js:2:7:2:12 | x | eval.js:4:3:4:3 | x |
| eval.js:2:11:2:12 | 42 | eval.js:2:7:2:12 | x |
| sources.js:1:6:1:6 | x | sources.js:1:6:1:6 | x |

7
javascript/ql/test/library-tests/DataFlow/getIntValue.expected
View File

@@ -1,3 +1,10 @@
| arguments.js:4:38:4:38 | 0 | 0 |
| arguments.js:5:35:5:35 | 1 | 1 |
| arguments.js:7:29:7:29 | 2 | 2 |
| arguments.js:9:37:9:37 | 0 | 0 |
| arguments.js:11:7:11:7 | 1 | 1 |
| arguments.js:11:10:11:10 | 2 | 2 |
| arguments.js:11:13:11:13 | 3 | 3 |
| eval.js:2:11:2:12 | 42 | 42 |
| sources.js:4:12:4:13 | 19 | 19 |
| sources.js:5:4:5:5 | 23 | 23 |

10
javascript/ql/test/library-tests/DataFlow/incomplete.expected
View File

@@ -1,3 +1,13 @@
| arguments.js:1:1:12:4 | exceptional return of (functi ... );\\n})() | call |
| arguments.js:1:2:12:1 | exceptional return of anonymous function | call |
| arguments.js:2:5:10:5 | exceptional return of function f | call |
| arguments.js:2:16:2:16 | x | call |
| arguments.js:4:28:4:39 | arguments[0] | heap |
| arguments.js:5:25:5:36 | arguments[1] | heap |
| arguments.js:7:24:7:30 | args[2] | heap |
| arguments.js:9:27:9:38 | arguments[0] | heap |
| arguments.js:11:5:11:14 | exceptional return of f(1, 2, 3) | call |
| arguments.js:11:5:11:14 | f(1, 2, 3) | call |
| eval.js:1:1:5:1 | exceptional return of function k | call |
| eval.js:2:7:2:12 | x | eval |
| eval.js:3:3:3:6 | eval | global |

1
javascript/ql/test/library-tests/DataFlow/parameters.expected
View File

@@ -1,3 +1,4 @@
| arguments.js:2:16:2:16 | x |
| sources.js:1:6:1:6 | x |
| sources.js:3:11:3:11 | x |
| sources.js:9:14:9:18 | array |

13
javascript/ql/test/library-tests/DataFlow/sources.expected
View File

@@ -1,3 +1,16 @@
| arguments.js:1:1:1:0 | this |
| arguments.js:1:1:12:4 | (functi ... );\\n})() |
| arguments.js:1:2:1:1 | this |
| arguments.js:1:2:12:1 | functio ... , 3);\\n} |
| arguments.js:2:5:2:4 | this |
| arguments.js:2:5:10:5 | functio ... ;\\n } |
| arguments.js:2:16:2:16 | x |
| arguments.js:4:28:4:39 | arguments[0] |
| arguments.js:5:25:5:36 | arguments[1] |
| arguments.js:7:24:7:30 | args[2] |
| arguments.js:8:21:8:22 | {} |
| arguments.js:9:27:9:38 | arguments[0] |
| arguments.js:11:5:11:14 | f(1, 2, 3) |
| eval.js:1:1:1:0 | this |
| eval.js:1:1:1:0 | this |
| eval.js:1:1:5:1 | functio ... eval`\\n} |