Skip to content

Commit 9ffc029

Browse files
committed
add file write model for express-fileupload mv
1 parent cfd2dcf commit 9ffc029

File tree

3 files changed

+44
-0
lines changed

3 files changed

+44
-0
lines changed

javascript/ql/lib/semmle/javascript/frameworks/ExpressModules.qll

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,3 +226,30 @@ module ExpressLibraries {
226226
predicate producesUserControlledObjects() { isJson() or isExtendedUrlEncoded() }
227227
}
228228
}
229+
230+
/**
231+
* Provides classes for working with the `express-fileupload` package (https://github.com/richardgirges/express-fileupload);
232+
*/
233+
module FileUpload {
234+
/** Gets a data flow node referring to `req.files`. */
235+
private DataFlow::SourceNode filesRef(Express::RequestSource req, DataFlow::TypeTracker t) {
236+
t.start() and
237+
result = req.ref().getAPropertyRead("files")
238+
or
239+
exists(DataFlow::TypeTracker t2 | result = filesRef(req, t2).track(t2, t))
240+
}
241+
242+
/**
243+
* A call to `req.files.<name>.mv`
244+
*/
245+
class Move extends FileSystemWriteAccess, DataFlow::MethodCallNode {
246+
Move() {
247+
exists(DataFlow::moduleImport("express-fileupload")) and
248+
this = filesRef(_, DataFlow::TypeTracker::end()).getAPropertyRead().getAMethodCall("mv")
249+
}
250+
251+
override DataFlow::Node getAPathArgument() { result = getArgument(0) }
252+
253+
override DataFlow::Node getADataNode() { none() }
254+
}
255+
}

javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1535,6 +1535,12 @@ nodes
15351535
| TaintedPath.js:214:35:214:38 | path |
15361536
| TaintedPath.js:214:35:214:38 | path |
15371537
| TaintedPath.js:214:35:214:38 | path |
1538+
| express.js:8:20:8:32 | req.query.bar |
1539+
| express.js:8:20:8:32 | req.query.bar |
1540+
| express.js:8:20:8:32 | req.query.bar |
1541+
| express.js:8:20:8:32 | req.query.bar |
1542+
| express.js:8:20:8:32 | req.query.bar |
1543+
| express.js:8:20:8:32 | req.query.bar |
15381544
| normalizedPaths.js:11:7:11:27 | path |
15391545
| normalizedPaths.js:11:7:11:27 | path |
15401546
| normalizedPaths.js:11:7:11:27 | path |
@@ -6321,6 +6327,7 @@ edges
63216327
| TaintedPath.js:211:24:211:30 | req.url | TaintedPath.js:211:14:211:37 | url.par ... , true) |
63226328
| TaintedPath.js:211:24:211:30 | req.url | TaintedPath.js:211:14:211:37 | url.par ... , true) |
63236329
| TaintedPath.js:211:24:211:30 | req.url | TaintedPath.js:211:14:211:37 | url.par ... , true) |
6330+
| express.js:8:20:8:32 | req.query.bar | express.js:8:20:8:32 | req.query.bar |
63246331
| normalizedPaths.js:11:7:11:27 | path | normalizedPaths.js:13:19:13:22 | path |
63256332
| normalizedPaths.js:11:7:11:27 | path | normalizedPaths.js:13:19:13:22 | path |
63266333
| normalizedPaths.js:11:7:11:27 | path | normalizedPaths.js:13:19:13:22 | path |
@@ -9638,6 +9645,7 @@ edges
96389645
| TaintedPath.js:212:31:212:34 | path | TaintedPath.js:211:24:211:30 | req.url | TaintedPath.js:212:31:212:34 | path | This path depends on $@. | TaintedPath.js:211:24:211:30 | req.url | a user-provided value |
96399646
| TaintedPath.js:213:45:213:48 | path | TaintedPath.js:211:24:211:30 | req.url | TaintedPath.js:213:45:213:48 | path | This path depends on $@. | TaintedPath.js:211:24:211:30 | req.url | a user-provided value |
96409647
| TaintedPath.js:214:35:214:38 | path | TaintedPath.js:211:24:211:30 | req.url | TaintedPath.js:214:35:214:38 | path | This path depends on $@. | TaintedPath.js:211:24:211:30 | req.url | a user-provided value |
9648+
| express.js:8:20:8:32 | req.query.bar | express.js:8:20:8:32 | req.query.bar | express.js:8:20:8:32 | req.query.bar | This path depends on $@. | express.js:8:20:8:32 | req.query.bar | a user-provided value |
96419649
| normalizedPaths.js:13:19:13:22 | path | normalizedPaths.js:11:14:11:27 | req.query.path | normalizedPaths.js:13:19:13:22 | path | This path depends on $@. | normalizedPaths.js:11:14:11:27 | req.query.path | a user-provided value |
96429650
| normalizedPaths.js:14:19:14:29 | './' + path | normalizedPaths.js:11:14:11:27 | req.query.path | normalizedPaths.js:14:19:14:29 | './' + path | This path depends on $@. | normalizedPaths.js:11:14:11:27 | req.query.path | a user-provided value |
96439651
| normalizedPaths.js:15:19:15:38 | path + '/index.html' | normalizedPaths.js:11:14:11:27 | req.query.path | normalizedPaths.js:15:19:15:38 | path + '/index.html' | This path depends on $@. | normalizedPaths.js:11:14:11:27 | req.query.path | a user-provided value |
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
var express = require("express"),
2+
fileUpload = require("express-fileupload");
3+
4+
let app = express();
5+
app.use(fileUpload());
6+
7+
app.get("/some/path", function (req, res) {
8+
req.files.foo.mv(req.query.bar);
9+
});

0 commit comments

Comments
 (0)