hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 11:15:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

JS: Better local flow through .pipe chaining

Browse Source
This commit is contained in:
Jason Reed
2019-02-22 09:56:44 -05:00
parent 23d37c7167
commit b0636dd410
4 changed files with 58 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
javascript/ql/src/semmle/javascript/security/dataflow/ZipSlip.qll
View File

@@ -33,6 +33,33 @@ module ZipSlip {
}
}
/**
* Holds if `node1` flows to `node2` in one step by virtue of
* `node2` being of the form `.pipe(node1)`. The reason this flow
* exists is that `.pipe` returns its argument to make chained
* stream operations work.
*/
predicate pipeStep(DataFlow::Node node1, DataFlow::MethodCallNode node2) {
node2.getMethodName() = "pipe" and
node1 = node2.getArgument(0)
}
/**
* Holds if `node1` flows to `node2` in one step including the assumption that
* `x` flows to `.pipe(x)`
*/
predicate stepsThroughPipe(DataFlow::Node node1, DataFlow::Node node2) {
DataFlow::localFlowStep(node1, node2) or pipeStep(node1, node2)
}
/**
* Holds if `node1` flows to `node2` including the assumption that
* `x` flows to `.pipe(x)`
*/
predicate flowsThroughPipe(DataFlow::Node node1, DataFlow::Node node2) {
stepsThroughPipe*(node1, node2)
}
/**
* An access to the filepath of an entry of a zipfile being extracted
* by npm module `unzip`. For example, in
@@ -50,12 +77,10 @@ module ZipSlip {
*/
class UnzipEntrySource extends Source {
UnzipEntrySource() {
exists(DataFlow::MethodCallNode pipe, DataFlow::MethodCallNode on |
pipe.getMethodName() = "pipe" and
pipe.getArgument(0).getALocalSource() = DataFlow::moduleImport("unzip")
.getAMemberCall("Parse") and
on = pipe.getAMemberCall("on") and
this = on.getCallback(1).getParameter(0).getAPropertyRead("path")
exists(DataFlow::SourceNode parsed |
flowsThroughPipe(DataFlow::moduleImport("unzip").getAMemberCall("Parse"), parsed)
and
this = parsed.getAMemberCall("on").getCallback(1).getParameter(0).getAPropertyRead("path")
)
}
}

8
javascript/ql/test/query-tests/Security/CWE-022/ZipSlip/ZipSlip.expected
View File

@@ -1,5 +1,13 @@
nodes
| ZipSlipBad2.js:5:9:5:46 | fileName |
| ZipSlipBad2.js:5:20:5:46 | 'output ... ry.path |
| ZipSlipBad2.js:5:37:5:46 | entry.path |
| ZipSlipBad2.js:6:22:6:29 | fileName |
| ZipSlipBad.js:8:37:8:46 | entry.path |
edges
| ZipSlipBad2.js:5:9:5:46 | fileName | ZipSlipBad2.js:6:22:6:29 | fileName |
| ZipSlipBad2.js:5:20:5:46 | 'output ... ry.path | ZipSlipBad2.js:5:9:5:46 | fileName |
| ZipSlipBad2.js:5:37:5:46 | entry.path | ZipSlipBad2.js:5:20:5:46 | 'output ... ry.path |
#select
| ZipSlipBad2.js:6:22:6:29 | fileName | ZipSlipBad2.js:5:37:5:46 | entry.path | ZipSlipBad2.js:6:22:6:29 | fileName | Unsanitized zip archive $@, which may contain '..', is used in a file system operation. | ZipSlipBad2.js:5:37:5:46 | entry.path | item path |
| ZipSlipBad.js:8:37:8:46 | entry.path | ZipSlipBad.js:8:37:8:46 | entry.path | ZipSlipBad.js:8:37:8:46 | entry.path | Unsanitized zip archive $@, which may contain '..', is used in a file system operation. | ZipSlipBad.js:8:37:8:46 | entry.path | item path |

8
javascript/ql/test/query-tests/Security/CWE-022/ZipSlip/ZipSlipBad2.js Normal file
View File

@@ -0,0 +1,8 @@
var fs = require('fs');
var unzip = require('unzip');
fs.readFile('path/to/archive.zip', function (err, zipContents) {
unzip.Parse(zipContents).on('entry', function (entry) {
var fileName = 'output/path/' + entry.path;
fs.writeFileSync(fileName, entry.contents);
});
});

11
javascript/ql/test/query-tests/Security/CWE-022/ZipSlip/externs.js Normal file
View File

@@ -0,0 +1,11 @@
/**
* @externs
*/
var fs = {};
/**
* @param {string} filename
* @param {*} data
* @return {void}
*/
fs.writeFileSync = function(filename, data) {};