hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 11:15:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #4574 from erik-krogh/jsdom

Browse Source 
Approved by asgerf
This commit is contained in:
CodeQL CI
2020-11-05 22:13:39 +00:00
committed by GitHub
parent a3894be1c5 e124ba66b4
commit a908e5938e
7 changed files with 66 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
javascript/ql/src/semmle/javascript/frameworks/ClientRequests.qll
View File

@@ -809,4 +809,19 @@ module ClientRequest {
override DataFlow::Node getADataNode() { none() }
}
/**
* A model of a URL request made using `jsdom.fromUrl()`.
*/
class JSDOMFromUrl extends ClientRequest::Range {
JSDOMFromUrl() {
this = API::moduleImport("jsdom").getMember("JSDOM").getMember("fromURL").getACall()
}
override DataFlow::Node getUrl() { result = getArgument(0) }
override DataFlow::Node getHost() { none() }
override DataFlow::Node getADataNode() { none() }
}
}

9
javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll
View File

@@ -151,7 +151,7 @@ module DomBasedXss {
* An expression whose value is interpreted as HTML
* and may be inserted into the DOM through a library.
*/
class LibrarySink extends Sink, DataFlow::ValueNode {
class LibrarySink extends Sink {
LibrarySink() {
// call to a jQuery method that interprets its argument as HTML
exists(JQuery::MethodCall call |
@@ -175,6 +175,13 @@ module DomBasedXss {
this = any(Handlebars::SafeString s).getAnArgument()
or
this = any(JQuery::MethodCall call | call.getMethodName() = "jGrowl").getArgument(0)
or
// A construction of a JSDOM object (server side DOM), where scripts are allowed.
exists(DataFlow::NewNode instance |
instance = API::moduleImport("jsdom").getMember("JSDOM").getInstance().getAnImmediateUse() and
this = instance.getArgument(0) and
instance.getOptionArgument(1, "runScripts").mayHaveStringValue("dangerously")
)
}
}

5
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected
View File

@@ -89,6 +89,9 @@ nodes
| classnames.js:15:47:15:63 | clsx(window.name) |
| classnames.js:15:52:15:62 | window.name |
| classnames.js:15:52:15:62 | window.name |
| express.js:7:15:7:33 | req.param("wobble") |
| express.js:7:15:7:33 | req.param("wobble") |
| express.js:7:15:7:33 | req.param("wobble") |
| jquery.js:2:7:2:40 | tainted |
| jquery.js:2:7:2:40 | tainted |
| jquery.js:2:17:2:33 | document.location |
@@ -685,6 +688,7 @@ edges
| classnames.js:15:47:15:63 | clsx(window.name) | classnames.js:15:31:15:78 | `<span ... <span>` |
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
| express.js:7:15:7:33 | req.param("wobble") | express.js:7:15:7:33 | req.param("wobble") |
| jquery.js:2:7:2:40 | tainted | jquery.js:7:20:7:26 | tainted |
| jquery.js:2:7:2:40 | tainted | jquery.js:8:28:8:34 | tainted |
| jquery.js:2:17:2:33 | document.location | jquery.js:2:17:2:40 | documen ... .search |
@@ -1175,6 +1179,7 @@ edges
| classnames.js:11:31:11:79 | `<span ... <span>` | classnames.js:10:45:10:55 | window.name | classnames.js:11:31:11:79 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:10:45:10:55 | window.name | user-provided value |
| classnames.js:13:31:13:83 | `<span ... <span>` | classnames.js:13:57:13:67 | window.name | classnames.js:13:31:13:83 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:13:57:13:67 | window.name | user-provided value |
| classnames.js:15:31:15:78 | `<span ... <span>` | classnames.js:15:52:15:62 | window.name | classnames.js:15:31:15:78 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:15:52:15:62 | window.name | user-provided value |
| express.js:7:15:7:33 | req.param("wobble") | express.js:7:15:7:33 | req.param("wobble") | express.js:7:15:7:33 | req.param("wobble") | Cross-site scripting vulnerability due to $@. | express.js:7:15:7:33 | req.param("wobble") | user-provided value |
| jquery.js:7:5:7:34 | "<div i ... + "\\">" | jquery.js:2:17:2:40 | documen ... .search | jquery.js:7:5:7:34 | "<div i ... + "\\">" | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:40 | documen ... .search | user-provided value |
| jquery.js:8:18:8:34 | "XSS: " + tainted | jquery.js:2:17:2:33 | document.location | jquery.js:8:18:8:34 | "XSS: " + tainted | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:33 | document.location | user-provided value |
| jquery.js:10:5:10:40 | "<b>" + ... "</b>" | jquery.js:10:13:10:20 | location | jquery.js:10:5:10:40 | "<b>" + ... "</b>" | Cross-site scripting vulnerability due to $@. | jquery.js:10:13:10:20 | location | user-provided value |

4
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected
View File

@@ -89,6 +89,9 @@ nodes
| classnames.js:15:47:15:63 | clsx(window.name) |
| classnames.js:15:52:15:62 | window.name |
| classnames.js:15:52:15:62 | window.name |
| express.js:7:15:7:33 | req.param("wobble") |
| express.js:7:15:7:33 | req.param("wobble") |
| express.js:7:15:7:33 | req.param("wobble") |
| jquery.js:2:7:2:40 | tainted |
| jquery.js:2:7:2:40 | tainted |
| jquery.js:2:17:2:33 | document.location |
@@ -689,6 +692,7 @@ edges
| classnames.js:15:47:15:63 | clsx(window.name) | classnames.js:15:31:15:78 | `<span ... <span>` |
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
| classnames.js:15:52:15:62 | window.name | classnames.js:15:47:15:63 | clsx(window.name) |
| express.js:7:15:7:33 | req.param("wobble") | express.js:7:15:7:33 | req.param("wobble") |
| jquery.js:2:7:2:40 | tainted | jquery.js:7:20:7:26 | tainted |
| jquery.js:2:7:2:40 | tainted | jquery.js:8:28:8:34 | tainted |
| jquery.js:2:17:2:33 | document.location | jquery.js:2:17:2:40 | documen ... .search |

11
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/express.js Normal file
View File

@@ -0,0 +1,11 @@
var express = require('express');
var app = express();
import { JSDOM } from "jsdom";
app.get('/some/path', function (req, res) {
// NOT OK
new JSDOM(req.param("wobble"), { runScripts: "dangerously" });
// OK
new JSDOM(req.param("wobble"), { runScripts: "outside-only" });
});

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

@@ -49,6 +49,14 @@ nodes
| tst.js:64:30:64:36 | tainted |
| tst.js:68:30:68:36 | tainted |
| tst.js:68:30:68:36 | tainted |
| tst.js:74:9:74:52 | tainted |
| tst.js:74:19:74:42 | url.par ... , true) |
| tst.js:74:19:74:48 | url.par ... ).query |
| tst.js:74:19:74:52 | url.par ... ery.url |
| tst.js:74:29:74:35 | req.url |
| tst.js:74:29:74:35 | req.url |
| tst.js:76:19:76:25 | tainted |
| tst.js:76:19:76:25 | tainted |
edges
| tst.js:14:9:14:52 | tainted | tst.js:18:13:18:19 | tainted |
| tst.js:14:9:14:52 | tainted | tst.js:18:13:18:19 | tainted |
@@ -98,6 +106,13 @@ edges
| tst.js:58:19:58:52 | url.par ... ery.url | tst.js:58:9:58:52 | tainted |
| tst.js:58:29:58:35 | req.url | tst.js:58:19:58:42 | url.par ... , true) |
| tst.js:58:29:58:35 | req.url | tst.js:58:19:58:42 | url.par ... , true) |
| tst.js:74:9:74:52 | tainted | tst.js:76:19:76:25 | tainted |
| tst.js:74:9:74:52 | tainted | tst.js:76:19:76:25 | tainted |
| tst.js:74:19:74:42 | url.par ... , true) | tst.js:74:19:74:48 | url.par ... ).query |
| tst.js:74:19:74:48 | url.par ... ).query | tst.js:74:19:74:52 | url.par ... ery.url |
| tst.js:74:19:74:52 | url.par ... ery.url | tst.js:74:9:74:52 | tainted |
| tst.js:74:29:74:35 | req.url | tst.js:74:19:74:42 | url.par ... , true) |
| tst.js:74:29:74:35 | req.url | tst.js:74:19:74:42 | url.par ... , true) |
#select
| 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 |
| 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
| 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 |
| 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 |
| 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 |
| 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 |

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

@@ -68,3 +68,10 @@ var server = http.createServer(async function(req, res) {
client.Page.navigate({url: tainted}); // NOT OK.
});
})
import {JSDOM} from "jsdom";
var server = http.createServer(async function(req, res) {
var tainted = url.parse(req.url, true).query.url;
JSDOM.fromURL(tainted); // NOT OK
});