hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-29 18:55:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

improve the markdown-it model

Browse Source
This commit is contained in:
Erik Krogh Kristensen
2021-04-20 15:23:03 +02:00
parent 19c5889775
commit 62dfd1fa7d
4 changed files with 76 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
javascript/ql/src/semmle/javascript/frameworks/Markdown.qll
View File

@@ -120,17 +120,41 @@ private class SnarkdownStep extends TaintTracking::SharedTaintStep {
}
/**
* A taint step for the `markdown-it` library.
* Classes and predicates for modelling taint steps the `markdown-it` library.
*/
private class MarkdownItStep extends TaintTracking::SharedTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists(API::CallNode renderer, API::CallNode call |
renderer = API::moduleImport("markdown-it").getACall() and
renderer.getParameter(0).getMember("html").getARhs().mayHaveBooleanValue(true) and
call = renderer.getReturn().getMember(["render", "renderInline"]).getACall()
private module MarkdownIt {
/**
* The creation of a parser from `markdown-it`.
*/
private API::Node markdownIt() {
exists(API::InvokeNode call |
call = API::moduleImport("markdown-it").getAnInvocation()
or
call = API::moduleImport("markdown-it").getMember("Markdown").getAnInvocation()
|
succ = call and
pred = call.getArgument(0)
call.getParameter(0).getMember("html").getARhs().mayHaveBooleanValue(true) and
result = call.getReturn()
)
or
exists(API::CallNode call |
call = markdownIt().getMember(["use", "set", "configure", "enable", "disable"]).getACall() and
result = call.getReturn() and
not call.getParameter(0).getARhs().getALocalSource() =
DataFlow::moduleImport("markdown-it-sanitizer")
)
}
/**
* A taint step for the `markdown-it` library.
*/
private class MarkdownItStep extends TaintTracking::SharedTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists(API::CallNode call |
call = markdownIt().getMember(["render", "renderInline"]).getACall()
|
succ = call and
pred = call.getArgument(0)
)
}
}
}

46
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected
View File

@@ -66,13 +66,21 @@ nodes
| ReflectedXss.js:85:12:85:31 | snarkdown2(req.body) |
| ReflectedXss.js:85:23:85:30 | req.body |
| ReflectedXss.js:85:23:85:30 | req.body |
| ReflectedXss.js:94:12:94:19 | req.body |
| ReflectedXss.js:94:12:94:19 | req.body |
| ReflectedXss.js:94:12:94:19 | req.body |
| ReflectedXss.js:95:12:95:38 | markdow ... q.body) |
| ReflectedXss.js:95:12:95:38 | markdow ... q.body) |
| ReflectedXss.js:95:30:95:37 | req.body |
| ReflectedXss.js:95:30:95:37 | req.body |
| ReflectedXss.js:97:12:97:19 | req.body |
| ReflectedXss.js:97:12:97:19 | req.body |
| ReflectedXss.js:97:12:97:19 | req.body |
| ReflectedXss.js:98:12:98:38 | markdow ... q.body) |
| ReflectedXss.js:98:12:98:38 | markdow ... q.body) |
| ReflectedXss.js:98:30:98:37 | req.body |
| ReflectedXss.js:98:30:98:37 | req.body |
| ReflectedXss.js:100:12:100:39 | markdow ... q.body) |
| ReflectedXss.js:100:12:100:39 | markdow ... q.body) |
| ReflectedXss.js:100:31:100:38 | req.body |
| ReflectedXss.js:100:31:100:38 | req.body |
| ReflectedXss.js:103:12:103:84 | markdow ... q.body) |
| ReflectedXss.js:103:12:103:84 | markdow ... q.body) |
| ReflectedXss.js:103:76:103:83 | req.body |
| ReflectedXss.js:103:76:103:83 | req.body |
| ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
| ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
| ReflectedXssContentTypes.js:10:24:10:36 | req.params.id |
@@ -219,11 +227,19 @@ edges
| ReflectedXss.js:85:23:85:30 | req.body | ReflectedXss.js:85:12:85:31 | snarkdown2(req.body) |
| ReflectedXss.js:85:23:85:30 | req.body | ReflectedXss.js:85:12:85:31 | snarkdown2(req.body) |
| ReflectedXss.js:85:23:85:30 | req.body | ReflectedXss.js:85:12:85:31 | snarkdown2(req.body) |
| ReflectedXss.js:94:12:94:19 | req.body | ReflectedXss.js:94:12:94:19 | req.body |
| ReflectedXss.js:95:30:95:37 | req.body | ReflectedXss.js:95:12:95:38 | markdow ... q.body) |
| ReflectedXss.js:95:30:95:37 | req.body | ReflectedXss.js:95:12:95:38 | markdow ... q.body) |
| ReflectedXss.js:95:30:95:37 | req.body | ReflectedXss.js:95:12:95:38 | markdow ... q.body) |
| ReflectedXss.js:95:30:95:37 | req.body | ReflectedXss.js:95:12:95:38 | markdow ... q.body) |
| ReflectedXss.js:97:12:97:19 | req.body | ReflectedXss.js:97:12:97:19 | req.body |
| ReflectedXss.js:98:30:98:37 | req.body | ReflectedXss.js:98:12:98:38 | markdow ... q.body) |
| ReflectedXss.js:98:30:98:37 | req.body | ReflectedXss.js:98:12:98:38 | markdow ... q.body) |
| ReflectedXss.js:98:30:98:37 | req.body | ReflectedXss.js:98:12:98:38 | markdow ... q.body) |
| ReflectedXss.js:98:30:98:37 | req.body | ReflectedXss.js:98:12:98:38 | markdow ... q.body) |
| ReflectedXss.js:100:31:100:38 | req.body | ReflectedXss.js:100:12:100:39 | markdow ... q.body) |
| ReflectedXss.js:100:31:100:38 | req.body | ReflectedXss.js:100:12:100:39 | markdow ... q.body) |
| ReflectedXss.js:100:31:100:38 | req.body | ReflectedXss.js:100:12:100:39 | markdow ... q.body) |
| ReflectedXss.js:100:31:100:38 | req.body | ReflectedXss.js:100:12:100:39 | markdow ... q.body) |
| ReflectedXss.js:103:76:103:83 | req.body | ReflectedXss.js:103:12:103:84 | markdow ... q.body) |
| ReflectedXss.js:103:76:103:83 | req.body | ReflectedXss.js:103:12:103:84 | markdow ... q.body) |
| ReflectedXss.js:103:76:103:83 | req.body | ReflectedXss.js:103:12:103:84 | markdow ... q.body) |
| ReflectedXss.js:103:76:103:83 | req.body | ReflectedXss.js:103:12:103:84 | markdow ... q.body) |
| ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
| ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
| ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
@@ -319,8 +335,10 @@ edges
| ReflectedXss.js:83:12:83:19 | req.body | ReflectedXss.js:83:12:83:19 | req.body | ReflectedXss.js:83:12:83:19 | req.body | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:83:12:83:19 | req.body | user-provided value |
| ReflectedXss.js:84:12:84:30 | snarkdown(req.body) | ReflectedXss.js:84:22:84:29 | req.body | ReflectedXss.js:84:12:84:30 | snarkdown(req.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:84:22:84:29 | req.body | user-provided value |
| ReflectedXss.js:85:12:85:31 | snarkdown2(req.body) | ReflectedXss.js:85:23:85:30 | req.body | ReflectedXss.js:85:12:85:31 | snarkdown2(req.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:85:23:85:30 | req.body | user-provided value |
| ReflectedXss.js:94:12:94:19 | req.body | ReflectedXss.js:94:12:94:19 | req.body | ReflectedXss.js:94:12:94:19 | req.body | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:94:12:94:19 | req.body | user-provided value |
| ReflectedXss.js:95:12:95:38 | markdow ... q.body) | ReflectedXss.js:95:30:95:37 | req.body | ReflectedXss.js:95:12:95:38 | markdow ... q.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:95:30:95:37 | req.body | user-provided value |
| ReflectedXss.js:97:12:97:19 | req.body | ReflectedXss.js:97:12:97:19 | req.body | ReflectedXss.js:97:12:97:19 | req.body | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:97:12:97:19 | req.body | user-provided value |
| ReflectedXss.js:98:12:98:38 | markdow ... q.body) | ReflectedXss.js:98:30:98:37 | req.body | ReflectedXss.js:98:12:98:38 | markdow ... q.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:98:30:98:37 | req.body | user-provided value |
| ReflectedXss.js:100:12:100:39 | markdow ... q.body) | ReflectedXss.js:100:31:100:38 | req.body | ReflectedXss.js:100:12:100:39 | markdow ... q.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:100:31:100:38 | req.body | user-provided value |
| ReflectedXss.js:103:12:103:84 | markdow ... q.body) | ReflectedXss.js:103:76:103:83 | req.body | ReflectedXss.js:103:12:103:84 | markdow ... q.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:103:76:103:83 | req.body | user-provided value |
| ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id | ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | user-provided value |
| ReflectedXssContentTypes.js:20:14:20:36 | "FOO: " ... rams.id | ReflectedXssContentTypes.js:20:24:20:36 | req.params.id | ReflectedXssContentTypes.js:20:14:20:36 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:20:24:20:36 | req.params.id | user-provided value |
| ReflectedXssContentTypes.js:39:13:39:35 | "FOO: " ... rams.id | ReflectedXssContentTypes.js:39:23:39:35 | req.params.id | ReflectedXssContentTypes.js:39:13:39:35 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:39:23:39:35 | req.params.id | user-provided value |

7
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.js
View File

@@ -90,8 +90,15 @@ const markdownIt = require('markdown-it')({
});
const markdownIt2 = require('markdown-it')({});
const markdownIt3 = require('markdown-it')({html: true})
.use(require('markdown-it-highlightjs'));
app.get('/user/:id', function (req, res) {
res.send(req.body); // NOT OK
res.send(markdownIt.render(req.body)); // NOT OK
res.send(markdownIt2.render(req.body)); // OK - no html
res.send(markdownIt3.render(req.body)); // NOT OK
res.send(markdownIt.use(require('markdown-it-sanitizer')).render(req.body)); // OK - HTML is sanitized.
res.send(markdownIt.use(require('markdown-it-abbr')).use(unknown).render(req.body)); // NOT OK
});

6
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected
View File

@@ -14,8 +14,10 @@
| ReflectedXss.js:83:12:83:19 | req.body | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:83:12:83:19 | req.body | user-provided value |
| ReflectedXss.js:84:12:84:30 | snarkdown(req.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:84:22:84:29 | req.body | user-provided value |
| ReflectedXss.js:85:12:85:31 | snarkdown2(req.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:85:23:85:30 | req.body | user-provided value |
| ReflectedXss.js:94:12:94:19 | req.body | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:94:12:94:19 | req.body | user-provided value |
| ReflectedXss.js:95:12:95:38 | markdow ... q.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:95:30:95:37 | req.body | user-provided value |
| ReflectedXss.js:97:12:97:19 | req.body | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:97:12:97:19 | req.body | user-provided value |
| ReflectedXss.js:98:12:98:38 | markdow ... q.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:98:30:98:37 | req.body | user-provided value |
| ReflectedXss.js:100:12:100:39 | markdow ... q.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:100:31:100:38 | req.body | user-provided value |
| ReflectedXss.js:103:12:103:84 | markdow ... q.body) | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:103:76:103:83 | req.body | user-provided value |
| ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | user-provided value |
| ReflectedXssContentTypes.js:20:14:20:36 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:20:24:20:36 | req.params.id | user-provided value |
| ReflectedXssContentTypes.js:39:13:39:35 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:39:23:39:35 | req.params.id | user-provided value |