Skip to content

Commit ca5f91e

Browse files
committed
recognize more startswith sanitizers for path-injection queries
1 parent 55e69d4 commit ca5f91e

File tree

2 files changed

+33
-3
lines changed

2 files changed

+33
-3
lines changed

javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -513,13 +513,24 @@ module TaintedPath {
513513

514514
override predicate blocks(boolean outcome, Expr e) {
515515
member = "relative" and
516-
e = pathCall.getArgument(1).asExpr() and
516+
e = maybeGetJoinArg(pathCall.getArgument(1)).asExpr() and
517517
outcome = startsWith.getPolarity().booleanNot()
518518
or
519519
not member = "relative" and
520-
e = pathCall.getArgument(0).asExpr() and
520+
e = maybeGetJoinArg(pathCall.getArgument(0)).asExpr() and
521521
outcome = startsWith.getPolarity()
522522
}
523+
524+
bindingset[e]
525+
private DataFlow::Node maybeGetJoinArg(DataFlow::Node e) {
526+
exists(DataFlow::CallNode call |
527+
call = NodeJSLib::Path::moduleMember("join").getACall() and e = call
528+
|
529+
result = call.getLastArgument()
530+
)
531+
or
532+
result = e
533+
}
523534
}
524535

525536
/**

javascript/ql/test/query-tests/Security/CWE-022/ZipSlip/ZipSlipBad.js

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ fs.createReadStream('archive.zip')
2525

2626
const JSZip = require('jszip');
2727
const zip = new JSZip();
28+
const path = require('path');
2829
function doZipSlip() {
2930
for (const name in zip.files) {
3031
fs.createWriteStream(name);
@@ -33,4 +34,22 @@ function doZipSlip() {
3334
zip.forEach((name, file) => {
3435
fs.createWriteStream(name);
3536
});
36-
}
37+
38+
const extractTo = path.resolve("/some/path/to/extract/to");
39+
var files = [];
40+
41+
for (var name in zip.files) {
42+
var entry = zip.files[name];
43+
44+
var targetPath = path.resolve(
45+
path.join(extractTo, name)
46+
);
47+
if (!targetPath.startsWith(extractTo)) {
48+
throw new Error("Entry is outside the extraction path");
49+
}
50+
files.push(name);
51+
}
52+
for (const file of files) {
53+
fs.createWriteStream(path.join(extractTo, file)); // OK
54+
}
55+
}

0 commit comments

Comments
 (0)