Skip to content

Commit 7046f1a

Browse files
erik-krogherik-krogh
committed
add taint-step for markdown-it when the HTML flag is set
1 parent 1f92390 commit 7046f1a

File tree

4 files changed

+44
-0
lines changed

4 files changed

+44
-0
lines changed

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,3 +118,20 @@ private class SnarkdownStep extends TaintTracking::SharedTaintStep {
118118
)
119119
}
120120
}
121+
122+
/**
123+
* A taint step for the `markdown-it` library.
124+
*/
125+
private class MarkdownItStep extends TaintTracking::SharedTaintStep {
126+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
127+
exists(API::CallNode renderer, API::CallNode call |
128+
renderer = API::moduleImport("markdown-it").getACall() and
129+
renderer.getParameter(0).getMember("html").getARhs().asExpr().(BooleanLiteral).getValue() =
130+
"true" and
131+
call = renderer.getReturn().getMember(["render", "renderInline"]).getACall()
132+
|
133+
succ = call and
134+
pred = call.getArgument(0)
135+
)
136+
}
137+
}

‎javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,13 @@ nodes
6666
| ReflectedXss.js:85:12:85:31 | snarkdown2(req.body) |
6767
| ReflectedXss.js:85:23:85:30 | req.body |
6868
| ReflectedXss.js:85:23:85:30 | req.body |
69+
| ReflectedXss.js:94:12:94:19 | req.body |
70+
| ReflectedXss.js:94:12:94:19 | req.body |
71+
| ReflectedXss.js:94:12:94:19 | req.body |
72+
| ReflectedXss.js:95:12:95:38 | markdow ... q.body) |
73+
| ReflectedXss.js:95:12:95:38 | markdow ... q.body) |
74+
| ReflectedXss.js:95:30:95:37 | req.body |
75+
| ReflectedXss.js:95:30:95:37 | req.body |
6976
| ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
7077
| ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
7178
| ReflectedXssContentTypes.js:10:24:10:36 | req.params.id |
@@ -212,6 +219,11 @@ edges
212219
| ReflectedXss.js:85:23:85:30 | req.body | ReflectedXss.js:85:12:85:31 | snarkdown2(req.body) |
213220
| ReflectedXss.js:85:23:85:30 | req.body | ReflectedXss.js:85:12:85:31 | snarkdown2(req.body) |
214221
| ReflectedXss.js:85:23:85:30 | req.body | ReflectedXss.js:85:12:85:31 | snarkdown2(req.body) |
222+
| ReflectedXss.js:94:12:94:19 | req.body | ReflectedXss.js:94:12:94:19 | req.body |
223+
| ReflectedXss.js:95:30:95:37 | req.body | ReflectedXss.js:95:12:95:38 | markdow ... q.body) |
224+
| ReflectedXss.js:95:30:95:37 | req.body | ReflectedXss.js:95:12:95:38 | markdow ... q.body) |
225+
| ReflectedXss.js:95:30:95:37 | req.body | ReflectedXss.js:95:12:95:38 | markdow ... q.body) |
226+
| ReflectedXss.js:95:30:95:37 | req.body | ReflectedXss.js:95:12:95:38 | markdow ... q.body) |
215227
| ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
216228
| ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
217229
| ReflectedXssContentTypes.js:10:24:10:36 | req.params.id | ReflectedXssContentTypes.js:10:14:10:36 | "FOO: " ... rams.id |
@@ -307,6 +319,8 @@ edges
307319
| 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 |
308320
| 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 |
309321
| 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 |
322+
| 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 |
323+
| 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 |
310324
| 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 |
311325
| 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 |
312326
| 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 |

‎javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,14 @@ app.get('/user/:id', function (req, res) {
8484
res.send(snarkdown(req.body)); // NOT OK
8585
res.send(snarkdown2(req.body)); // NOT OK
8686
});
87+
88+
const markdownIt = require('markdown-it')({
89+
html: true
90+
});
91+
const markdownIt2 = require('markdown-it')({});
92+
93+
app.get('/user/:id', function (req, res) {
94+
res.send(req.body); // NOT OK
95+
res.send(markdownIt.render(req.body)); // NOT OK
96+
res.send(markdownIt2.render(req.body)); // OK - no html
97+
});

‎javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
| 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 |
1515
| 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 |
1616
| 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 |
17+
| 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 |
18+
| 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 |
1719
| 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 |
1820
| 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 |
1921
| 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 |

0 commit comments

Comments
 (0)