Skip to content

JS: torrent content as a remote flow source #2324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
semmle-qlci merged 2 commits into from
Nov 14, 2019
Merged

JS: torrent content as a remote flow source #2324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bea59ec
JS: add some parsed torrent properties as remote flow sources
esbena Nov 13, 2019
cc76834
JS: add security tests for malicious torrents
esbena Nov 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions change-notes/1.23/analysis-javascript.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- [firebase](https://www.npmjs.com/package/firebase)
- [mongodb](https://www.npmjs.com/package/mongodb)
- [mongoose](https://www.npmjs.com/package/mongoose)
- [parse-torrent](https://github.com/webtorrent/parse-torrent)
- [rate-limiter-flexible](https://www.npmjs.com/package/rate-limiter-flexible)

* The call graph has been improved to resolve method calls in more cases. This may produce more security alerts.
Expand Down
1 change: 1 addition & 0 deletions javascript/ql/src/javascript.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ import semmle.javascript.frameworks.SystemCommandExecutors
import semmle.javascript.frameworks.SQL
import semmle.javascript.frameworks.SocketIO
import semmle.javascript.frameworks.StringFormatters
import semmle.javascript.frameworks.TorrentLibraries
import semmle.javascript.frameworks.UriLibraries
import semmle.javascript.frameworks.Vue
import semmle.javascript.frameworks.XmlParsers
Expand Down
61 changes: 61 additions & 0 deletions javascript/ql/src/semmle/javascript/frameworks/TorrentLibraries.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* Provides classes for modelling Torrent libraries.
*/

import javascript

/**
* Provides classes for working with [parse-torrent](https://github.com/webtorrent/parse-torrent) code.
*/
module ParseTorrent {
private DataFlow::SourceNode mod() { result = DataFlow::moduleImport("parse-torrent") }

/**
* A torrent that has been parsed into a JavaScript object.
*/
class ParsedTorrent extends DataFlow::SourceNode {
ParsedTorrent() {
this = mod().getACall() or
this = mod().getAMemberCall("remote").getCallback(1).getParameter(1)
}
}

private DataFlow::SourceNode parsedTorrentRef(DataFlow::TypeTracker t) {
t.start() and
result instanceof ParsedTorrent
or
exists(DataFlow::TypeTracker t2 | result = parsedTorrentRef(t2).track(t2, t))
}

/** Gets a data flow node referring to a parsed torrent. */
DataFlow::SourceNode parsedTorrentRef() {
result = parsedTorrentRef(DataFlow::TypeTracker::end())
}

/**
* An access to user-controlled torrent information.
*/
class UserControlledTorrentInfo extends RemoteFlowSource {
UserControlledTorrentInfo() {
exists(DataFlow::SourceNode ref, DataFlow::PropRead read |
ref = parsedTorrentRef() and
read = ref.getAPropertyRead() and
this = read
|
exists(string prop |
not (
prop = "private" or
prop = "infoHash" or
prop = "length"
// "pieceLength" and "lastPieceLength" are not guaranteed to be numbers as of commit ae3ad15d
) and
read.getPropertyName() = prop
)
or
not exists(read.getPropertyName())
)
}

override string getSourceType() { result = "torrent information" }
}
}
5 changes: 5 additions & 0 deletions javascript/ql/src/semmle/javascript/security/dataflow/StoredXss.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}
1 change: 1 addition & 0 deletions javascript/ql/test/library-tests/TorrentLibraries/ParsedTorrent.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
| tst.js:4:15:4:28 | parseTorrent() |
3 changes: 3 additions & 0 deletions javascript/ql/test/library-tests/TorrentLibraries/ParsedTorrent.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import javascript

select any(ParseTorrent::ParsedTorrent t)
4 changes: 4 additions & 0 deletions javascript/ql/test/library-tests/TorrentLibraries/UserControlledTorrentInfo.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
| tst.js:6:2:6:12 | parsed.name |
| tst.js:8:2:8:19 | parsed.pieceLength |
| tst.js:14:2:14:25 | indirec ... ed.name |
| tst.js:20:2:20:7 | t.name |
3 changes: 3 additions & 0 deletions javascript/ql/test/library-tests/TorrentLibraries/UserControlledTorrentInfo.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import javascript

select any(ParseTorrent::UserControlledTorrentInfo i)
3 changes: 3 additions & 0 deletions javascript/ql/test/library-tests/TorrentLibraries/parsedTorrentRef.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
| tst.js:4:15:4:28 | parseTorrent() |
| tst.js:14:2:14:20 | indirection1.parsed |
| tst.js:19:23:19:23 | t |
3 changes: 3 additions & 0 deletions javascript/ql/test/library-tests/TorrentLibraries/parsedTorrentRef.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import javascript

select ParseTorrent::parsedTorrentRef()
21 changes: 21 additions & 0 deletions javascript/ql/test/library-tests/TorrentLibraries/tst.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
const parseTorrent = require('parse-torrent');

(function(){
let parsed = parseTorrent();

parsed.name;
parsed.length;
parsed.pieceLength;

let indirection1 = {
parsed: parsed
};

indirection1.parsed.name;
indirection2(parsed);

});

function indirection2(t) {
t.name;
}
42 changes: 42 additions & 0 deletions javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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] |
Expand Down Expand Up @@ -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 |
Expand Down Expand Up @@ -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 |
8 changes: 8 additions & 0 deletions javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/torrents.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
}
10 changes: 10 additions & 0 deletions javascript/ql/test/query-tests/Security/CWE-079/StoredXss.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -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 |
8 changes: 8 additions & 0 deletions javascript/ql/test/query-tests/Security/CWE-079/xss-through-torrent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
});