diff --git a/javascript/ql/src/Security/CWE-022/TaintedPath.qhelp b/javascript/ql/src/Security/CWE-022/TaintedPath.qhelp index 8dab2fcc4fc6..1b0e3c033084 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 validation method you should use 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. +You should always normalize the file path since an unnormalized path that starts with the root folder can still be used to access files outside the root folder. +Then, after you have normalized the path, check that the path starts with 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 first (bad) example, the code reads the file name from an HTTP request, then accesses that file within a root folder. +A malicious user could enter a file name containing "../" segments to navigate outside the root folder and access sensitive files.

+ +

-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. +The second (good) example shows how to avoid access to sensitive files by sanitizing the file path. +First, the code resolves the file name relative to a root folder, normalizing the path and removing any "../" segments in the process. +Then, the code calls fs.realpathSync to resolve any symbolic links in the path. +Finally, the code checks that the normalized path starts with the path of the root folder, ensuring 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 daa641193ff3..1fdbef68c47e 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'); -var server = http.createServer(function(req, res) { - let path = url.parse(req.url, true).query.path; +const ROOT = "/var/www/"; - // BAD: This could read any file on the file system - res.write(fs.readFileSync(path)); +var server = http.createServer(function(req, res) { + let filePath = url.parse(req.url, true).query.path; - // BAD: This could still read any file on the file system - res.write(fs.readFileSync("/home/user/" + path)); -}); + // BAD: This function uses unsanitized input that can 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/src/Security/CWE-022/examples/TaintedPathGood.js b/javascript/ql/src/Security/CWE-022/examples/TaintedPathGood.js new file mode 100644 index 000000000000..ac8dd4fb9ba8 --- /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 2d1692dce00e..177d6b266ebf 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 000000000000..1fdbef68c47e --- /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 function uses unsanitized input that can 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 000000000000..ac8dd4fb9ba8 --- /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