Skip to content

Commit f6e0ccf

Browse files
asger-semmleasger-semmle
committed
JS: model URI and XHR methods from closure library
1 parent fd2e9f1 commit f6e0ccf

File tree

7 files changed

+176
-6
lines changed

7 files changed

+176
-6
lines changed

‎javascript/ql/src/semmle/javascript/Closure.qll‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,5 @@ module Closure {
198198
/**
199199
* Gets a dataflow node that refers to the given value exported from a Closure module.
200200
*/
201-
DataFlow::SourceNode moduleImport(string moduleName) {
202-
getLibraryAccessPath(result) = moduleName
203-
}
201+
DataFlow::SourceNode moduleImport(string moduleName) { getLibraryAccessPath(result) = moduleName }
204202
}

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,11 +240,38 @@ private class SuperAgentUrlRequest extends CustomClientRequest {
240240
* A model of a URL request made using the `XMLHttpRequest` browser class.
241241
*/
242242
private class XMLHttpRequest extends CustomClientRequest {
243-
XMLHttpRequest() { this = DataFlow::globalVarRef("XMLHttpRequest").getAnInstantiation() }
243+
XMLHttpRequest() {
244+
this = DataFlow::globalVarRef("XMLHttpRequest").getAnInstantiation()
245+
or
246+
// closure shim for XMLHttpRequest
247+
this = Closure::moduleImport("goog.net.XmlHttp").getAnInstantiation()
248+
}
244249

245250
override DataFlow::Node getUrl() { result = getAMethodCall("open").getArgument(1) }
246251

247252
override DataFlow::Node getHost() { none() }
248253

249254
override DataFlow::Node getADataNode() { result = getAMethodCall("send").getArgument(0) }
250255
}
256+
257+
/**
258+
* A model of a URL request made using the `XhrIo` class from the closure library.
259+
*/
260+
private class ClosureXhrIoRequest extends CustomClientRequest {
261+
ClosureXhrIoRequest() {
262+
exists(DataFlow::SourceNode xhrIo | xhrIo = Closure::moduleImport("goog.net.XhrIo") |
263+
this = xhrIo.getAMethodCall("send")
264+
or
265+
this = xhrIo.getAnInstantiation().getAMethodCall("send")
266+
)
267+
}
268+
269+
override DataFlow::Node getUrl() { result = getArgument(0) }
270+
271+
override DataFlow::Node getHost() { none() }
272+
273+
override DataFlow::Node getADataNode() {
274+
result = getArgument(2) or
275+
result = getArgument(3)
276+
}
277+
}

‎javascript/ql/src/semmle/javascript/frameworks/UriLibraries.qll‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,3 +309,88 @@ module querystring {
309309
override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this }
310310
}
311311
}
312+
313+
/**
314+
* Provides steps for the `goog.Uri` class in the closure library.
315+
*/
316+
private module ClosureLibraryUri {
317+
/**
318+
* Taint step from an argument of a `goog.Uri` call to the return value.
319+
*/
320+
private class ArgumentStep extends UriLibraryStep, DataFlow::InvokeNode {
321+
int arg;
322+
323+
ArgumentStep() {
324+
// goog.Uri constructor
325+
this = Closure::moduleImport("goog.Uri").getAnInstantiation() and arg = 0
326+
or
327+
// static methods on goog.Uri
328+
exists(string name | this = Closure::moduleImport("goog.Uri." + name).getACall() |
329+
name = "parse" and arg = 0
330+
or
331+
name = "create" and
332+
(arg = 0 or arg = 2 or arg = 4)
333+
or
334+
name = "resolve" and
335+
(arg = 0 or arg = 1)
336+
)
337+
or
338+
// static methods in goog.uri.utils
339+
arg = 0 and
340+
exists(string name | this = Closure::moduleImport("goog.uri.utils." + name).getACall() |
341+
name = "appendParam" or // preserve taint from the original URI, but not from the appended param
342+
name = "appendParams" or
343+
name = "appendParamsFromMap" or
344+
name = "appendPath" or
345+
name = "getParamValue" or
346+
name = "getParamValues" or
347+
name = "getPath" or
348+
name = "getPathAndAfter" or
349+
name = "getQueryData" or
350+
name = "parseQueryData" or
351+
name = "removeFragment" or
352+
name = "removeParam" or
353+
name = "setParam" or
354+
name = "setParamsFromMap" or
355+
name = "setPath" or
356+
name = "split"
357+
)
358+
}
359+
360+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
361+
pred = getArgument(arg) and
362+
succ = this
363+
}
364+
}
365+
366+
/**
367+
* Taint steps through chainable setter calls.
368+
*
369+
* Setters mutate the URI object and return the same instance.
370+
*/
371+
private class SetterCall extends DataFlow::MethodCallNode, UriLibraryStep {
372+
DataFlow::NewNode uri;
373+
string name;
374+
375+
SetterCall() {
376+
exists(DataFlow::SourceNode base |
377+
base = Closure::moduleImport("goog.Uri").getAnInstantiation() and
378+
uri = base
379+
or
380+
base.(SetterCall).getUri() = uri
381+
|
382+
this = base.getAMethodCall(name) and
383+
name.matches("set%")
384+
)
385+
}
386+
387+
DataFlow::NewNode getUri() { result = uri }
388+
389+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
390+
pred = getReceiver() and succ = this
391+
or
392+
(name = "setDomain" or name = "setPath" or name = "setScheme") and
393+
pred = getArgument(0) and succ = uri
394+
}
395+
}
396+
}

‎javascript/ql/test/library-tests/frameworks/UriLibraries/UriLibraryStep.expected‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,27 @@
1+
| closureUri.js:5:11:5:20 | new Uri(x) | closureUri.js:5:19:5:19 | x | closureUri.js:5:11:5:20 | new Uri(x) |
2+
| closureUri.js:6:1:6:12 | Uri.parse(x) | closureUri.js:6:11:6:11 | x | closureUri.js:6:1:6:12 | Uri.parse(x) |
3+
| closureUri.js:7:1:7:17 | Uri.resolve(x, y) | closureUri.js:7:13:7:13 | x | closureUri.js:7:1:7:17 | Uri.resolve(x, y) |
4+
| closureUri.js:7:1:7:17 | Uri.resolve(x, y) | closureUri.js:7:16:7:16 | y | closureUri.js:7:1:7:17 | Uri.resolve(x, y) |
5+
| closureUri.js:8:1:8:57 | Uri.cre ... , frag) | closureUri.js:8:12:8:17 | scheme | closureUri.js:8:1:8:57 | Uri.cre ... , frag) |
6+
| closureUri.js:8:1:8:57 | Uri.cre ... , frag) | closureUri.js:8:26:8:31 | domain | closureUri.js:8:1:8:57 | Uri.cre ... , frag) |
7+
| closureUri.js:8:1:8:57 | Uri.cre ... , frag) | closureUri.js:8:40:8:43 | path | closureUri.js:8:1:8:57 | Uri.cre ... , frag) |
8+
| closureUri.js:10:1:10:16 | uri.setScheme(x) | closureUri.js:10:1:10:3 | uri | closureUri.js:10:1:10:16 | uri.setScheme(x) |
9+
| closureUri.js:10:1:10:16 | uri.setScheme(x) | closureUri.js:10:15:10:15 | x | closureUri.js:5:11:5:20 | new Uri(x) |
10+
| closureUri.js:11:1:11:18 | uri.setUserInfo(x) | closureUri.js:11:1:11:3 | uri | closureUri.js:11:1:11:18 | uri.setUserInfo(x) |
11+
| closureUri.js:12:1:12:16 | uri.setDomain(x) | closureUri.js:12:1:12:3 | uri | closureUri.js:12:1:12:16 | uri.setDomain(x) |
12+
| closureUri.js:12:1:12:16 | uri.setDomain(x) | closureUri.js:12:15:12:15 | x | closureUri.js:5:11:5:20 | new Uri(x) |
13+
| closureUri.js:13:1:13:14 | uri.setPort(x) | closureUri.js:13:1:13:3 | uri | closureUri.js:13:1:13:14 | uri.setPort(x) |
14+
| closureUri.js:14:1:14:14 | uri.setPath(x) | closureUri.js:14:1:14:3 | uri | closureUri.js:14:1:14:14 | uri.setPath(x) |
15+
| closureUri.js:14:1:14:14 | uri.setPath(x) | closureUri.js:14:13:14:13 | x | closureUri.js:5:11:5:20 | new Uri(x) |
16+
| closureUri.js:15:1:15:15 | uri.setQuery(x) | closureUri.js:15:1:15:3 | uri | closureUri.js:15:1:15:15 | uri.setQuery(x) |
17+
| closureUri.js:16:1:16:18 | uri.setFragment(x) | closureUri.js:16:1:16:3 | uri | closureUri.js:16:1:16:18 | uri.setFragment(x) |
18+
| closureUri.js:18:1:18:15 | uri.setQuery(x) | closureUri.js:18:1:18:3 | uri | closureUri.js:18:1:18:15 | uri.setQuery(x) |
19+
| closureUri.js:18:1:18:26 | uri.set ... Path(y) | closureUri.js:18:1:18:15 | uri.setQuery(x) | closureUri.js:18:1:18:26 | uri.set ... Path(y) |
20+
| closureUri.js:18:1:18:26 | uri.set ... Path(y) | closureUri.js:18:25:18:25 | y | closureUri.js:5:11:5:20 | new Uri(x) |
21+
| closureUri.js:18:1:18:39 | uri.set ... heme(z) | closureUri.js:18:1:18:26 | uri.set ... Path(y) | closureUri.js:18:1:18:39 | uri.set ... heme(z) |
22+
| closureUri.js:18:1:18:39 | uri.set ... heme(z) | closureUri.js:18:38:18:38 | z | closureUri.js:5:11:5:20 | new Uri(x) |
23+
| closureUri.js:22:1:22:25 | utils.a ... uri, z) | closureUri.js:22:19:22:21 | uri | closureUri.js:22:1:22:25 | utils.a ... uri, z) |
24+
| closureUri.js:23:1:23:18 | utils.getPath(uri) | closureUri.js:23:15:23:17 | uri | closureUri.js:23:1:23:18 | utils.getPath(uri) |
125
| punycode.js:3:9:3:26 | punycode.decode(x) | punycode.js:3:25:3:25 | x | punycode.js:3:9:3:26 | punycode.decode(x) |
226
| punycode.js:5:5:5:22 | punycode.encode(x) | punycode.js:5:21:5:21 | x | punycode.js:5:5:5:22 | punycode.encode(x) |
327
| punycode.js:7:5:7:25 | punycod ... code(x) | punycode.js:7:24:7:24 | x | punycode.js:7:5:7:25 | punycod ... code(x) |

‎javascript/ql/test/library-tests/frameworks/UriLibraries/closureUri.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
goog.module('closureUri');
2+
3+
let Uri = goog.require('goog.Uri');
4+
5+
let uri = new Uri(x);
6+
Uri.parse(x);
7+
Uri.resolve(x, y);
8+
Uri.create(scheme, cred, domain, port, path, query, frag);
9+
10+
uri.setScheme(x);
11+
uri.setUserInfo(x);
12+
uri.setDomain(x);
13+
uri.setPort(x);
14+
uri.setPath(x);
15+
uri.setQuery(x);
16+
uri.setFragment(x);
17+
18+
uri.setQuery(x).setPath(y).setScheme(z);
19+
20+
let utils = goog.require('goog.uri.utils');
21+
22+
utils.appendParam(uri, z);
23+
utils.getPath(uri);

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ nodes
1414
| tst.js:30:13:30:43 | "http:/ ... tainted |
1515
| tst.js:30:37:30:43 | tainted |
1616
| tst.js:34:34:34:40 | tainted |
17+
| tst.js:36:16:36:31 | new Uri(tainted) |
18+
| tst.js:36:24:36:30 | tainted |
19+
| tst.js:37:22:37:37 | new Uri(tainted) |
20+
| tst.js:37:30:37:36 | tainted |
1721
edges
1822
| tst.js:14:9:14:52 | tainted | tst.js:18:13:18:19 | tainted |
1923
| tst.js:14:9:14:52 | tainted | tst.js:20:17:20:23 | tainted |
@@ -22,13 +26,17 @@ edges
2226
| tst.js:14:9:14:52 | tainted | tst.js:28:36:28:42 | tainted |
2327
| tst.js:14:9:14:52 | tainted | tst.js:30:37:30:43 | tainted |
2428
| tst.js:14:9:14:52 | tainted | tst.js:34:34:34:40 | tainted |
29+
| tst.js:14:9:14:52 | tainted | tst.js:36:24:36:30 | tainted |
30+
| tst.js:14:9:14:52 | tainted | tst.js:37:30:37:36 | tainted |
2531
| tst.js:14:19:14:42 | url.par ... , true) | tst.js:14:19:14:48 | url.par ... ).query |
2632
| tst.js:14:19:14:48 | url.par ... ).query | tst.js:14:19:14:52 | url.par ... ery.url |
2733
| tst.js:14:19:14:52 | url.par ... ery.url | tst.js:14:9:14:52 | tainted |
2834
| tst.js:14:29:14:35 | req.url | tst.js:14:19:14:42 | url.par ... , true) |
2935
| tst.js:26:25:26:31 | tainted | tst.js:26:13:26:31 | "http://" + tainted |
3036
| tst.js:28:36:28:42 | tainted | tst.js:28:13:28:42 | "http:/ ... tainted |
3137
| tst.js:30:37:30:43 | tainted | tst.js:30:13:30:43 | "http:/ ... tainted |
38+
| tst.js:36:24:36:30 | tainted | tst.js:36:16:36:31 | new Uri(tainted) |
39+
| tst.js:37:30:37:36 | tainted | tst.js:37:22:37:37 | new Uri(tainted) |
3240
#select
3341
| 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 |
3442
| 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 |
@@ -37,3 +45,5 @@ edges
3745
| tst.js:28:5:28:43 | request ... ainted) | tst.js:14:29:14:35 | req.url | tst.js:28:13:28:42 | "http:/ ... tainted | The $@ of this request depends on $@. | tst.js:28:13:28:42 | "http:/ ... tainted | URL | tst.js:14:29:14:35 | req.url | a user-provided value |
3846
| tst.js:30:5:30:44 | request ... ainted) | tst.js:14:29:14:35 | req.url | tst.js:30:13:30:43 | "http:/ ... tainted | The $@ of this request depends on $@. | tst.js:30:13:30:43 | "http:/ ... tainted | URL | tst.js:14:29:14:35 | req.url | a user-provided value |
3947
| tst.js:34:5:34:42 | http.ge ... inted}) | tst.js:14:29:14:35 | req.url | tst.js:34:34:34:40 | tainted | The $@ of this request depends on $@. | tst.js:34:34:34:40 | tainted | host | tst.js:14:29:14:35 | req.url | a user-provided value |
48+
| tst.js:36:5:36:32 | XhrIo.s ... inted)) | tst.js:14:29:14:35 | req.url | tst.js:36:16:36:31 | new Uri(tainted) | The $@ of this request depends on $@. | tst.js:36:16:36:31 | new Uri(tainted) | URL | tst.js:14:29:14:35 | req.url | a user-provided value |
49+
| tst.js:37:5:37:38 | new Xhr ... inted)) | tst.js:14:29:14:35 | req.url | tst.js:37:22:37:37 | new Uri(tainted) | The $@ of this request depends on $@. | tst.js:37:22:37:37 | new Uri(tainted) | URL | tst.js:14:29:14:35 | req.url | a user-provided value |

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import axios from 'axios';
77
import got from 'got';
88
import nodeFetch from 'node-fetch';
99
import url from 'url';
10-
11-
10+
let XhrIo = goog.require('goog.net.XhrIo');
11+
let Uri = goog.require('goog.Uri');
1212

1313
var server = http.createServer(function(req, res) {
1414
var tainted = url.parse(req.url, true).query.url;
@@ -32,4 +32,7 @@ var server = http.createServer(function(req, res) {
3232
request("http://example.com/?" + tainted); // OK
3333

3434
http.get(relativeUrl, {host: tainted}); // NOT OK
35+
36+
XhrIo.send(new Uri(tainted)); // NOT OK
37+
new XhrIo().send(new Uri(tainted)); // NOT OK
3538
})

0 commit comments

Comments
 (0)