From d147faba4e892ba27e16f3e8b4d66786a0570b94 Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Thu, 16 Nov 2023 11:55:28 +0000 Subject: [PATCH] Update qhelp for js/path-injection. --- .../ql/src/Security/CWE-022/TaintedPath.qhelp | 51 +++--- .../Security/CWE-022/examples/TaintedPath.js | 17 +- .../CWE-022/examples/TaintedPathGood.js | 19 ++ .../CWE-022/TaintedPath/TaintedPath.expected | 167 ++++++++++++++++++ .../TaintedPath/examples/TaintedPath.js | 12 ++ .../TaintedPath/examples/TaintedPathGood.js | 19 ++ 6 files changed, 254 insertions(+), 31 deletions(-) create mode 100644 javascript/ql/src/Security/CWE-022/examples/TaintedPathGood.js create mode 100644 javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/examples/TaintedPath.js create mode 100644 javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/examples/TaintedPathGood.js diff --git a/javascript/ql/src/Security/CWE-022/TaintedPath.qhelp b/javascript/ql/src/Security/CWE-022/TaintedPath.qhelp index 8dab2fcc4fc..5517f2c7ed6 100644 --- a/javascript/ql/src/Security/CWE-022/TaintedPath.qhelp +++ b/javascript/ql/src/Security/CWE-022/TaintedPath.qhelp @@ -13,40 +13,47 @@ attacker being able to influence behavior by modifying unexpected files.

-Validate user input before using it to construct a file path, either using an off-the-shelf library -like the sanitize-filename npm package, or by performing custom validation. +Validate user input before using it to construct a file path.

-Ideally, follow these rules: +The choice of validation depends on whether you want to allow the user to specify complex paths with multiple components that may span multiple folders, or only simple filenames without a path component.

- +

+In the former case, a common strategy is to make sure that the constructed file path is contained within a safe root folder. +First, normalize the path using path.resolve or fs.realpathSync to remove any ".." segments. +Then check that the normalized path starts with the root folder. +Note that the normalization step is important, since otherwise even a path that starts with the root folder could be used to access files outside the root folder. +

+ +

+In the latter case, you can use a library like the sanitize-filename npm package to eliminate any special characters from the file path. +Note that it is not sufficient to only remove "../" sequences: for example, applying this filter to ".../...//" would still result in the string "../". +

+ +

+Finally, the simplest (but most restrictive) option is to use an allow list of safe patterns and make sure that the user input matches one of these patterns. +

-In the first example, a file name is read from an HTTP request and then used to access a file. -However, a malicious user could enter a file name which is an absolute path, such as -"/etc/passwd". -

- -

-In the second example, it appears that the user is restricted to opening a file within the -"user" home directory. However, a malicious user could enter a file name containing -special characters. For example, the string "../../etc/passwd" will result in the code -reading the file located at "/home/user/../../etc/passwd", which is the system's -password file. This file would then be sent back to the user, giving them access to all the -system's passwords. +In the first example, a file name is read from an HTTP request and then used to access a file within a root folder. +However, a malicious user could enter a file name containing "../" segments to navigate outside the root folder and access sensitive files.

+ +

+The second example shows how to fix this. +First, the file name is resolved relative to a root folder, which has the side effect of normalizing the path and removing any "../" segments. +Then, fs.realpathSync is used to resolve any symbolic links in the path. +Finally, we check that the normalized path starts with the root folder's path, which ensures that the file is contained within the root folder. +

+ + +
diff --git a/javascript/ql/src/Security/CWE-022/examples/TaintedPath.js b/javascript/ql/src/Security/CWE-022/examples/TaintedPath.js index daa641193ff..768bd51cb13 100644 --- a/javascript/ql/src/Security/CWE-022/examples/TaintedPath.js +++ b/javascript/ql/src/Security/CWE-022/examples/TaintedPath.js @@ -1,13 +1,12 @@ -var fs = require('fs'), - http = require('http'), - url = require('url'); +const fs = require('fs'), + http = require('http'), + url = require('url'); + +const ROOT = "/var/www/"; var server = http.createServer(function(req, res) { - let path = url.parse(req.url, true).query.path; + let filePath = url.parse(req.url, true).query.path; // BAD: This could read any file on the file system - res.write(fs.readFileSync(path)); - - // BAD: This could still read any file on the file system - res.write(fs.readFileSync("/home/user/" + path)); -}); + res.write(fs.readFileSync(ROOT + filePath, 'utf8')); +}); \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-022/examples/TaintedPathGood.js b/javascript/ql/src/Security/CWE-022/examples/TaintedPathGood.js new file mode 100644 index 00000000000..ac8dd4fb9ba --- /dev/null +++ b/javascript/ql/src/Security/CWE-022/examples/TaintedPathGood.js @@ -0,0 +1,19 @@ +const fs = require('fs'), + http = require('http'), + path = require('path'), + url = require('url'); + +const ROOT = "/var/www/"; + +var server = http.createServer(function(req, res) { + let filePath = url.parse(req.url, true).query.path; + + // GOOD: Verify that the file path is under the root directory + filePath = fs.realpathSync(path.resolve(ROOT, filePath)); + if (!filePath.startsWith(ROOT)) { + res.statusCode = 403; + res.end(); + return; + } + res.write(fs.readFileSync(filePath, 'utf8')); +}); \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected index 2d1692dce00..177d6b266eb 100644 --- a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected +++ b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected @@ -1535,6 +1535,76 @@ nodes | TaintedPath.js:214:35:214:38 | path | | TaintedPath.js:214:35:214:38 | path | | TaintedPath.js:214:35:214:38 | path | +| examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:28:8:34 | req.url | +| examples/TaintedPath.js:8:28:8:34 | req.url | +| examples/TaintedPath.js:8:28:8:34 | req.url | +| examples/TaintedPath.js:8:28:8:34 | req.url | +| examples/TaintedPath.js:8:28:8:34 | req.url | +| examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | | 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 | @@ -6635,6 +6705,102 @@ edges | TaintedPath.js:211:24:211:30 | req.url | TaintedPath.js:211:14:211:37 | url.par ... , true) | | TaintedPath.js:211:24:211:30 | req.url | TaintedPath.js:211:14:211:37 | url.par ... , true) | | TaintedPath.js:211:24:211:30 | req.url | TaintedPath.js:211:14:211:37 | url.par ... , true) | +| examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:11:36:11:43 | filePath | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | +| examples/TaintedPath.js:11:36:11:43 | filePath | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | | express.js:8:20:8:32 | req.query.bar | express.js:8:20:8:32 | req.query.bar | | handlebars.js:10:51:10:58 | filePath | handlebars.js:11:32:11:39 | filePath | | handlebars.js:10:51:10:58 | filePath | handlebars.js:11:32:11:39 | filePath | @@ -10345,6 +10511,7 @@ edges | 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 a $@. | TaintedPath.js:211:24:211:30 | req.url | user-provided value | | 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 a $@. | TaintedPath.js:211:24:211:30 | req.url | user-provided value | | 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 a $@. | TaintedPath.js:211:24:211:30 | req.url | user-provided value | +| examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:11:29:11:43 | ROOT + filePath | This path depends on a $@. | examples/TaintedPath.js:8:28:8:34 | req.url | user-provided value | | 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 a $@. | express.js:8:20:8:32 | req.query.bar | user-provided value | | handlebars.js:11:32:11:39 | filePath | handlebars.js:29:46:29:60 | req.params.path | handlebars.js:11:32:11:39 | filePath | This path depends on a $@. | handlebars.js:29:46:29:60 | req.params.path | user-provided value | | handlebars.js:15:25:15:32 | filePath | handlebars.js:43:15:43:29 | req.params.path | handlebars.js:15:25:15:32 | filePath | This path depends on a $@. | handlebars.js:43:15:43:29 | req.params.path | user-provided value | diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/examples/TaintedPath.js b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/examples/TaintedPath.js new file mode 100644 index 00000000000..768bd51cb13 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/examples/TaintedPath.js @@ -0,0 +1,12 @@ +const fs = require('fs'), + http = require('http'), + url = require('url'); + +const ROOT = "/var/www/"; + +var server = http.createServer(function(req, res) { + let filePath = url.parse(req.url, true).query.path; + + // BAD: This could read any file on the file system + res.write(fs.readFileSync(ROOT + filePath, 'utf8')); +}); \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/examples/TaintedPathGood.js b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/examples/TaintedPathGood.js new file mode 100644 index 00000000000..ac8dd4fb9ba --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/examples/TaintedPathGood.js @@ -0,0 +1,19 @@ +const fs = require('fs'), + http = require('http'), + path = require('path'), + url = require('url'); + +const ROOT = "/var/www/"; + +var server = http.createServer(function(req, res) { + let filePath = url.parse(req.url, true).query.path; + + // GOOD: Verify that the file path is under the root directory + filePath = fs.realpathSync(path.resolve(ROOT, filePath)); + if (!filePath.startsWith(ROOT)) { + res.statusCode = 403; + res.end(); + return; + } + res.write(fs.readFileSync(filePath, 'utf8')); +}); \ No newline at end of file