Skip to content

Commit 3815797

Browse files
committed
add sanitizers from DOM and jQuery queries
1 parent 8ba5bdd commit 3815797

File tree

4 files changed

+30
-3
lines changed

4 files changed

+30
-3
lines changed

javascript/ql/src/semmle/javascript/security/dataflow/UnsafeHtmlConstruction.qll

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,13 @@ module UnsafeHtmlConstruction {
2323

2424
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
2525

26-
override predicate isSanitizer(DataFlow::Node node) { super.isSanitizer(node) }
26+
override predicate isSanitizer(DataFlow::Node node) {
27+
super.isSanitizer(node)
28+
or
29+
node instanceof DomBasedXss::Sanitizer
30+
or
31+
node instanceof UnsafeJQueryPlugin::Sanitizer
32+
}
2733

2834
override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
2935
DomBasedXss::isOptionallySanitizedEdge(pred, succ)

javascript/ql/src/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ module UnsafeHtmlConstruction {
1313
private import semmle.javascript.security.dataflow.DomBasedXssCustomizations::DomBasedXss as DomBasedXss
1414
private import semmle.javascript.security.dataflow.UnsafeJQueryPluginCustomizations::UnsafeJQueryPlugin as UnsafeJQueryPlugin
1515
private import semmle.javascript.PackageExports as Exports
16-
private import semmle.javascript.security.dataflow.UnsafeJQueryPlugin::UnsafeJQueryPlugin as UnsafeJQueryPlugin
1716

1817
/**
1918
* A source for unsafe HTML constructed from library input.

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ nodes
3333
| main.js:62:19:62:26 | settings |
3434
| main.js:62:19:62:31 | settings.name |
3535
| main.js:62:19:62:31 | settings.name |
36+
| main.js:66:35:66:41 | attrVal |
37+
| main.js:66:35:66:41 | attrVal |
38+
| main.js:67:63:67:69 | attrVal |
39+
| main.js:67:63:67:69 | attrVal |
3640
| typed.ts:1:39:1:39 | s |
3741
| typed.ts:1:39:1:39 | s |
3842
| typed.ts:2:29:2:29 | s |
@@ -82,6 +86,10 @@ edges
8286
| main.js:60:41:60:47 | options | main.js:60:22:60:48 | $.exten ... ptions) |
8387
| main.js:62:19:62:26 | settings | main.js:62:19:62:31 | settings.name |
8488
| main.js:62:19:62:26 | settings | main.js:62:19:62:31 | settings.name |
89+
| main.js:66:35:66:41 | attrVal | main.js:67:63:67:69 | attrVal |
90+
| main.js:66:35:66:41 | attrVal | main.js:67:63:67:69 | attrVal |
91+
| main.js:66:35:66:41 | attrVal | main.js:67:63:67:69 | attrVal |
92+
| main.js:66:35:66:41 | attrVal | main.js:67:63:67:69 | attrVal |
8593
| typed.ts:1:39:1:39 | s | typed.ts:2:29:2:29 | s |
8694
| typed.ts:1:39:1:39 | s | typed.ts:2:29:2:29 | s |
8795
| typed.ts:1:39:1:39 | s | typed.ts:2:29:2:29 | s |
@@ -104,5 +112,6 @@ edges
104112
| main.js:22:34:22:34 | s | main.js:21:47:21:47 | s | main.js:22:34:22:34 | s | $@ based on $@ might later cause $@. | main.js:22:34:22:34 | s | Markdown rendering | main.js:21:47:21:47 | s | library input | main.js:23:53:23:56 | html | cross-site scripting |
105113
| main.js:47:65:47:73 | this.step | main.js:52:41:52:41 | s | main.js:47:65:47:73 | this.step | $@ based on $@ might later cause $@. | main.js:47:65:47:73 | this.step | HTML construction | main.js:52:41:52:41 | s | library input | main.js:47:54:47:85 | "<span> ... /span>" | cross-site scripting |
106114
| main.js:62:19:62:31 | settings.name | main.js:56:28:56:34 | options | main.js:62:19:62:31 | settings.name | $@ based on $@ might later cause $@. | main.js:62:19:62:31 | settings.name | HTML construction | main.js:56:28:56:34 | options | library input | main.js:62:11:62:40 | "<b>" + ... "</b>" | cross-site scripting |
115+
| main.js:67:63:67:69 | attrVal | main.js:66:35:66:41 | attrVal | main.js:67:63:67:69 | attrVal | $@ based on $@ might later cause $@. | main.js:67:63:67:69 | attrVal | HTML construction | main.js:66:35:66:41 | attrVal | library input | main.js:67:47:67:78 | "<img a ... "\\"/>" | cross-site scripting |
107116
| typed.ts:2:29:2:29 | s | typed.ts:1:39:1:39 | s | typed.ts:2:29:2:29 | s | $@ based on $@ might later cause $@. | typed.ts:2:29:2:29 | s | HTML construction | typed.ts:1:39:1:39 | s | library input | typed.ts:3:31:3:34 | html | cross-site scripting |
108117
| typed.ts:8:40:8:40 | s | typed.ts:6:43:6:43 | s | typed.ts:8:40:8:40 | s | $@ based on $@ might later cause $@. | typed.ts:8:40:8:40 | s | HTML construction | typed.ts:6:43:6:43 | s | library input | typed.ts:8:29:8:52 | "<span> ... /span>" | cross-site scripting |

javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/main.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,17 @@ $.fn.xssPlugin = function (options) {
6161
return this.each(function () {
6262
$("<b>" + settings.name + "</b>").appendTo(this); // NOT OK
6363
});
64-
}
64+
}
65+
66+
module.exports.guards = function (attrVal) {
67+
document.querySelector("#id").innerHTML = "<img alt=\"" + attrVal + "\"/>"; // NOT OK
68+
document.querySelector("#id").innerHTML = "<img alt=\"" + attrVal.replace(/"|'/g, "") + "\"/>"; // OK
69+
if (attrVal.indexOf("\"") === -1 && attrVal.indexOf("'") === -1) {
70+
document.querySelector("#id").innerHTML = "<img alt=\"" + attrVal + "\"/>"; // OK
71+
}
72+
}
73+
74+
module.exports.intentionalTemplate = function (obj) {
75+
const html = "<span>" + obj.spanTemplate + "</span>"; // OK
76+
document.querySelector("#template").innerHTML = html;
77+
}

0 commit comments

Comments
 (0)