From cc768345d0ae324f8fc836233805b7b6938432f7 Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Thu, 14 Nov 2019 13:52:15 +0100 Subject: [PATCH] JS: add security tests for malicious torrents --- .../security/dataflow/StoredXss.qll | 5 +++ .../CWE-022/TaintedPath/TaintedPath.expected | 42 +++++++++++++++++++ .../Security/CWE-022/TaintedPath/torrents.js | 8 ++++ .../Security/CWE-079/StoredXss.expected | 10 +++++ .../Security/CWE-079/xss-through-torrent.js | 8 ++++ 5 files changed, 73 insertions(+) create mode 100644 javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/torrents.js create mode 100644 javascript/ql/test/query-tests/Security/CWE-079/xss-through-torrent.js diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/StoredXss.qll b/javascript/ql/src/semmle/javascript/security/dataflow/StoredXss.qll index b6855a5d7b4..7741d1e8778 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/StoredXss.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/StoredXss.qll @@ -28,4 +28,9 @@ module StoredXss { class FileNameSourceAsSource extends Source { FileNameSourceAsSource() { this instanceof FileNameSource } } + + /** User-controlled torrent information, considered as a flow source for stored XSS. */ + class UserControlledTorrentInfoAsSource extends Source { + UserControlledTorrentInfoAsSource() { this instanceof ParseTorrent::UserControlledTorrentInfo } + } } 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 ad39e2200be..056f020660c 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 @@ -1183,6 +1183,26 @@ nodes | tainted-sendFile.js:25:34:25:45 | req.params.x | | tainted-sendFile.js:25:34:25:45 | req.params.x | | tainted-sendFile.js:25:34:25:45 | req.params.x | +| torrents.js:5:6:5:38 | name | +| torrents.js:5:6:5:38 | name | +| torrents.js:5:6:5:38 | name | +| torrents.js:5:13:5:38 | parseTo ... t).name | +| torrents.js:5:13:5:38 | parseTo ... t).name | +| torrents.js:5:13:5:38 | parseTo ... t).name | +| torrents.js:5:13:5:38 | parseTo ... t).name | +| torrents.js:6:6:6:45 | loc | +| torrents.js:6:6:6:45 | loc | +| torrents.js:6:6:6:45 | loc | +| torrents.js:6:12:6:45 | dir + " ... t.data" | +| torrents.js:6:12:6:45 | dir + " ... t.data" | +| torrents.js:6:12:6:45 | dir + " ... t.data" | +| torrents.js:6:24:6:27 | name | +| torrents.js:6:24:6:27 | name | +| torrents.js:6:24:6:27 | name | +| torrents.js:7:25:7:27 | loc | +| torrents.js:7:25:7:27 | loc | +| torrents.js:7:25:7:27 | loc | +| torrents.js:7:25:7:27 | loc | | views.js:1:43:1:55 | req.params[0] | | views.js:1:43:1:55 | req.params[0] | | views.js:1:43:1:55 | req.params[0] | @@ -2910,6 +2930,27 @@ edges | tainted-sendFile.js:25:34:25:45 | req.params.x | tainted-sendFile.js:25:16:25:46 | path.jo ... rams.x) | | tainted-sendFile.js:25:34:25:45 | req.params.x | tainted-sendFile.js:25:16:25:46 | path.jo ... rams.x) | | tainted-sendFile.js:25:34:25:45 | req.params.x | tainted-sendFile.js:25:16:25:46 | path.jo ... rams.x) | +| torrents.js:5:6:5:38 | name | torrents.js:6:24:6:27 | name | +| torrents.js:5:6:5:38 | name | torrents.js:6:24:6:27 | name | +| torrents.js:5:6:5:38 | name | torrents.js:6:24:6:27 | name | +| torrents.js:5:13:5:38 | parseTo ... t).name | torrents.js:5:6:5:38 | name | +| torrents.js:5:13:5:38 | parseTo ... t).name | torrents.js:5:6:5:38 | name | +| torrents.js:5:13:5:38 | parseTo ... t).name | torrents.js:5:6:5:38 | name | +| torrents.js:5:13:5:38 | parseTo ... t).name | torrents.js:5:6:5:38 | name | +| torrents.js:5:13:5:38 | parseTo ... t).name | torrents.js:5:6:5:38 | name | +| torrents.js:5:13:5:38 | parseTo ... t).name | torrents.js:5:6:5:38 | name | +| torrents.js:6:6:6:45 | loc | torrents.js:7:25:7:27 | loc | +| torrents.js:6:6:6:45 | loc | torrents.js:7:25:7:27 | loc | +| torrents.js:6:6:6:45 | loc | torrents.js:7:25:7:27 | loc | +| torrents.js:6:6:6:45 | loc | torrents.js:7:25:7:27 | loc | +| torrents.js:6:6:6:45 | loc | torrents.js:7:25:7:27 | loc | +| torrents.js:6:6:6:45 | loc | torrents.js:7:25:7:27 | loc | +| torrents.js:6:12:6:45 | dir + " ... t.data" | torrents.js:6:6:6:45 | loc | +| torrents.js:6:12:6:45 | dir + " ... t.data" | torrents.js:6:6:6:45 | loc | +| torrents.js:6:12:6:45 | dir + " ... t.data" | torrents.js:6:6:6:45 | loc | +| torrents.js:6:24:6:27 | name | torrents.js:6:12:6:45 | dir + " ... t.data" | +| torrents.js:6:24:6:27 | name | torrents.js:6:12:6:45 | dir + " ... t.data" | +| torrents.js:6:24:6:27 | name | torrents.js:6:12:6:45 | dir + " ... t.data" | | views.js:1:43:1:55 | req.params[0] | views.js:1:43:1:55 | req.params[0] | #select | TaintedPath-es6.js:10:26:10:45 | join("public", path) | TaintedPath-es6.js:7:20:7:26 | req.url | TaintedPath-es6.js:10:26:10:45 | join("public", path) | This path depends on $@. | TaintedPath-es6.js:7:20:7:26 | req.url | a user-provided value | @@ -2981,4 +3022,5 @@ edges | tainted-sendFile.js:18:43:18:58 | req.param("dir") | tainted-sendFile.js:18:43:18:58 | req.param("dir") | tainted-sendFile.js:18:43:18:58 | req.param("dir") | This path depends on $@. | tainted-sendFile.js:18:43:18:58 | req.param("dir") | a user-provided value | | tainted-sendFile.js:24:16:24:49 | path.re ... rams.x) | tainted-sendFile.js:24:37:24:48 | req.params.x | tainted-sendFile.js:24:16:24:49 | path.re ... rams.x) | This path depends on $@. | tainted-sendFile.js:24:37:24:48 | req.params.x | a user-provided value | | tainted-sendFile.js:25:16:25:46 | path.jo ... rams.x) | tainted-sendFile.js:25:34:25:45 | req.params.x | tainted-sendFile.js:25:16:25:46 | path.jo ... rams.x) | This path depends on $@. | tainted-sendFile.js:25:34:25:45 | req.params.x | a user-provided value | +| torrents.js:7:25:7:27 | loc | torrents.js:5:13:5:38 | parseTo ... t).name | torrents.js:7:25:7:27 | loc | This path depends on $@. | torrents.js:5:13:5:38 | parseTo ... t).name | a user-provided value | | views.js:1:43:1:55 | req.params[0] | views.js:1:43:1:55 | req.params[0] | views.js:1:43:1:55 | req.params[0] | This path depends on $@. | views.js:1:43:1:55 | req.params[0] | a user-provided value | diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/torrents.js b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/torrents.js new file mode 100644 index 00000000000..1e95cf84ec7 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/torrents.js @@ -0,0 +1,8 @@ +const parseTorrent = require('parse-torrent'), + fs = require('fs'); + +function getTorrentData(dir, torrent){ + let name = parseTorrent(torrent).name, + loc = dir + "/" + name + ".torrent.data"; + return fs.readFileSync(loc); // NOT OK +} diff --git a/javascript/ql/test/query-tests/Security/CWE-079/StoredXss.expected b/javascript/ql/test/query-tests/Security/CWE-079/StoredXss.expected index b0062e70b78..d6142c980b6 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/StoredXss.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/StoredXss.expected @@ -19,6 +19,11 @@ nodes | xss-through-filenames.js:35:29:35:34 | files2 | | xss-through-filenames.js:37:19:37:24 | files3 | | xss-through-filenames.js:37:19:37:24 | files3 | +| xss-through-torrent.js:6:6:6:24 | name | +| xss-through-torrent.js:6:13:6:24 | torrent.name | +| xss-through-torrent.js:6:13:6:24 | torrent.name | +| xss-through-torrent.js:7:11:7:14 | name | +| xss-through-torrent.js:7:11:7:14 | name | edges | xss-through-filenames.js:7:43:7:48 | files1 | xss-through-filenames.js:8:18:8:23 | files1 | | xss-through-filenames.js:7:43:7:48 | files1 | xss-through-filenames.js:8:18:8:23 | files1 | @@ -41,8 +46,13 @@ edges | xss-through-filenames.js:35:13:35:35 | files3 | xss-through-filenames.js:37:19:37:24 | files3 | | xss-through-filenames.js:35:22:35:35 | format(files2) | xss-through-filenames.js:35:13:35:35 | files3 | | xss-through-filenames.js:35:29:35:34 | files2 | xss-through-filenames.js:35:22:35:35 | format(files2) | +| xss-through-torrent.js:6:6:6:24 | name | xss-through-torrent.js:7:11:7:14 | name | +| xss-through-torrent.js:6:6:6:24 | name | xss-through-torrent.js:7:11:7:14 | name | +| xss-through-torrent.js:6:13:6:24 | torrent.name | xss-through-torrent.js:6:6:6:24 | name | +| xss-through-torrent.js:6:13:6:24 | torrent.name | xss-through-torrent.js:6:6:6:24 | name | #select | xss-through-filenames.js:8:18:8:23 | files1 | xss-through-filenames.js:7:43:7:48 | files1 | xss-through-filenames.js:8:18:8:23 | files1 | Stored cross-site scripting vulnerability due to $@. | xss-through-filenames.js:7:43:7:48 | files1 | stored value | | xss-through-filenames.js:26:19:26:24 | files1 | xss-through-filenames.js:25:43:25:48 | files1 | xss-through-filenames.js:26:19:26:24 | files1 | Stored cross-site scripting vulnerability due to $@. | xss-through-filenames.js:25:43:25:48 | files1 | stored value | | xss-through-filenames.js:33:19:33:24 | files2 | xss-through-filenames.js:25:43:25:48 | files1 | xss-through-filenames.js:33:19:33:24 | files2 | Stored cross-site scripting vulnerability due to $@. | xss-through-filenames.js:25:43:25:48 | files1 | stored value | | xss-through-filenames.js:37:19:37:24 | files3 | xss-through-filenames.js:25:43:25:48 | files1 | xss-through-filenames.js:37:19:37:24 | files3 | Stored cross-site scripting vulnerability due to $@. | xss-through-filenames.js:25:43:25:48 | files1 | stored value | +| xss-through-torrent.js:7:11:7:14 | name | xss-through-torrent.js:6:13:6:24 | torrent.name | xss-through-torrent.js:7:11:7:14 | name | Stored cross-site scripting vulnerability due to $@. | xss-through-torrent.js:6:13:6:24 | torrent.name | stored value | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/xss-through-torrent.js b/javascript/ql/test/query-tests/Security/CWE-079/xss-through-torrent.js new file mode 100644 index 00000000000..dcf530e3ef5 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-079/xss-through-torrent.js @@ -0,0 +1,8 @@ +const parseTorrent = require('parse-torrent'), + express = require('express'); + +express().get('/user/:id', function(req, res) { + let torrent = parseTorrent(unknown), + name = torrent.name; + res.send(name); // NOT OK +});