Skip to content

Commit e6e4a48

Browse files
committed
add JSDOM.fromUrl() as a request forgery sink
1 parent ce00d58 commit e6e4a48

File tree

3 files changed

+38
-0
lines changed

3 files changed

+38
-0
lines changed

javascript/ql/src/semmle/javascript/frameworks/ClientRequests.qll

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -809,4 +809,19 @@ module ClientRequest {
809809

810810
override DataFlow::Node getADataNode() { none() }
811811
}
812+
813+
/**
814+
* A model of a URL request made using `jsdom.fromUrl()`.
815+
*/
816+
class JSDOMFromUrl extends ClientRequest::Range {
817+
JSDOMFromUrl() {
818+
this = API::moduleImport("jsdom").getMember("JSDOM").getMember("fromURL").getACall()
819+
}
820+
821+
override DataFlow::Node getUrl() { result = getArgument(0) }
822+
823+
override DataFlow::Node getHost() { none() }
824+
825+
override DataFlow::Node getADataNode() { none() }
826+
}
812827
}

javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@ nodes
4949
| tst.js:64:30:64:36 | tainted |
5050
| tst.js:68:30:68:36 | tainted |
5151
| tst.js:68:30:68:36 | tainted |
52+
| tst.js:74:9:74:52 | tainted |
53+
| tst.js:74:19:74:42 | url.par ... , true) |
54+
| tst.js:74:19:74:48 | url.par ... ).query |
55+
| tst.js:74:19:74:52 | url.par ... ery.url |
56+
| tst.js:74:29:74:35 | req.url |
57+
| tst.js:74:29:74:35 | req.url |
58+
| tst.js:76:19:76:25 | tainted |
59+
| tst.js:76:19:76:25 | tainted |
5260
edges
5361
| tst.js:14:9:14:52 | tainted | tst.js:18:13:18:19 | tainted |
5462
| tst.js:14:9:14:52 | tainted | tst.js:18:13:18:19 | tainted |
@@ -98,6 +106,13 @@ edges
98106
| tst.js:58:19:58:52 | url.par ... ery.url | tst.js:58:9:58:52 | tainted |
99107
| tst.js:58:29:58:35 | req.url | tst.js:58:19:58:42 | url.par ... , true) |
100108
| tst.js:58:29:58:35 | req.url | tst.js:58:19:58:42 | url.par ... , true) |
109+
| tst.js:74:9:74:52 | tainted | tst.js:76:19:76:25 | tainted |
110+
| tst.js:74:9:74:52 | tainted | tst.js:76:19:76:25 | tainted |
111+
| tst.js:74:19:74:42 | url.par ... , true) | tst.js:74:19:74:48 | url.par ... ).query |
112+
| tst.js:74:19:74:48 | url.par ... ).query | tst.js:74:19:74:52 | url.par ... ery.url |
113+
| tst.js:74:19:74:52 | url.par ... ery.url | tst.js:74:9:74:52 | tainted |
114+
| tst.js:74:29:74:35 | req.url | tst.js:74:19:74:42 | url.par ... , true) |
115+
| tst.js:74:29:74:35 | req.url | tst.js:74:19:74:42 | url.par ... , true) |
101116
#select
102117
| tst.js:18:5:18:20 | request(tainted) | tst.js:14:29:14:35 | req.url | tst.js:18:13:18:19 | tainted | The $@ of this request depends on $@. | tst.js:18:13:18:19 | tainted | URL | tst.js:14:29:14:35 | req.url | a user-provided value |
103118
| tst.js:20:5:20:24 | request.get(tainted) | tst.js:14:29:14:35 | req.url | tst.js:20:17:20:23 | tainted | The $@ of this request depends on $@. | tst.js:20:17:20:23 | tainted | URL | tst.js:14:29:14:35 | req.url | a user-provided value |
@@ -114,3 +129,4 @@ edges
114129
| tst.js:61:2:61:37 | client. ... inted}) | tst.js:58:29:58:35 | req.url | tst.js:61:29:61:35 | tainted | The $@ of this request depends on $@. | tst.js:61:29:61:35 | tainted | URL | tst.js:58:29:58:35 | req.url | a user-provided value |
115130
| tst.js:64:3:64:38 | client. ... inted}) | tst.js:58:29:58:35 | req.url | tst.js:64:30:64:36 | tainted | The $@ of this request depends on $@. | tst.js:64:30:64:36 | tainted | URL | tst.js:58:29:58:35 | req.url | a user-provided value |
116131
| tst.js:68:3:68:38 | client. ... inted}) | tst.js:58:29:58:35 | req.url | tst.js:68:30:68:36 | tainted | The $@ of this request depends on $@. | tst.js:68:30:68:36 | tainted | URL | tst.js:58:29:58:35 | req.url | a user-provided value |
132+
| tst.js:76:5:76:26 | JSDOM.f ... ainted) | tst.js:74:29:74:35 | req.url | tst.js:76:19:76:25 | tainted | The $@ of this request depends on $@. | tst.js:76:19:76:25 | tainted | URL | tst.js:74:29:74:35 | req.url | a user-provided value |

javascript/ql/test/query-tests/Security/CWE-918/tst.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,10 @@ var server = http.createServer(async function(req, res) {
6868
client.Page.navigate({url: tainted}); // NOT OK.
6969
});
7070
})
71+
72+
import {JSDOM} from "jsdom";
73+
var server = http.createServer(async function(req, res) {
74+
var tainted = url.parse(req.url, true).query.url;
75+
76+
JSDOM.fromURL(tainted); // NOT OK
77+
});

0 commit comments

Comments
 (0)