Skip to content

Commit 85b9003

Browse files
esbenaesbena
committed
JS: add Mootools XSS sinks
1 parent 89cea5c commit 85b9003

File tree

7 files changed

+123
-1
lines changed

7 files changed

+123
-1
lines changed

‎javascript/change-notes/2021-06-30-mootools.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
lgtm,codescanning
2+
* HTML properties in the MooTools library are now recognized as sinks for `js/xss`.
3+
Affected packages are
4+
[Mootools](https://mootools.net/)

‎javascript/ql/src/javascript.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ import semmle.javascript.frameworks.Logging
103103
import semmle.javascript.frameworks.HttpFrameworks
104104
import semmle.javascript.frameworks.HttpProxy
105105
import semmle.javascript.frameworks.Markdown
106+
import semmle.javascript.frameworks.MooTools
106107
import semmle.javascript.frameworks.Nest
107108
import semmle.javascript.frameworks.Next
108109
import semmle.javascript.frameworks.NoSQL

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Provides classes for working with MooTools code.
3+
*/
4+
5+
import javascript
6+
7+
module MooTools {
8+
private class Element extends DataFlow::NewNode {
9+
Element() {
10+
this = DataFlow::globalVarRef("Element").getAnInstantiation() and
11+
// sharpen slightly to avoid spurious matches for the global variable
12+
this.getNumArgument() = [1, 2]
13+
}
14+
15+
DataFlow::Node getAnElementPropertyValue(string name) {
16+
result = this.getOptionArgument(1, name)
17+
or
18+
exists(DataFlow::MethodCallNode mcn |
19+
mcn = this.getAMethodCall(["set", "setProperty"]) and
20+
mcn.getArgument(0).mayHaveStringValue(name) and
21+
result = mcn.getArgument(1)
22+
or
23+
mcn = this.getAMethodCall(["set", "setProperties"]) and
24+
result = mcn.getOptionArgument(0, name)
25+
)
26+
}
27+
}
28+
29+
/**
30+
* Holds if MooTools interprets `node` as HTML.
31+
*/
32+
predicate interpretsNodeAsHtml(DataFlow::Node node) {
33+
exists(Element e |
34+
node = e.getAnElementPropertyValue("html") or
35+
node = e.getAMethodCall(["appendHtml"]).getArgument(0)
36+
)
37+
}
38+
}

‎javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,8 @@ module DomBasedXss {
192192
this = instance.getArgument(0) and
193193
instance.getOptionArgument(1, "runScripts").mayHaveStringValue("dangerously")
194194
)
195+
or
196+
MooTools::interpretsNodeAsHtml(this)
195197
}
196198
}
197199

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -669,6 +669,21 @@ nodes
669669
| tst.js:430:18:430:23 | target |
670670
| tst.js:430:18:430:89 | target. ... data>') |
671671
| tst.js:430:18:430:89 | target. ... data>') |
672+
| tst.js:436:6:436:38 | source |
673+
| tst.js:436:15:436:38 | documen ... .search |
674+
| tst.js:436:15:436:38 | documen ... .search |
675+
| tst.js:440:28:440:33 | source |
676+
| tst.js:440:28:440:33 | source |
677+
| tst.js:441:33:441:38 | source |
678+
| tst.js:441:33:441:38 | source |
679+
| tst.js:442:34:442:39 | source |
680+
| tst.js:442:34:442:39 | source |
681+
| tst.js:443:41:443:46 | source |
682+
| tst.js:443:41:443:46 | source |
683+
| tst.js:444:44:444:49 | source |
684+
| tst.js:444:44:444:49 | source |
685+
| tst.js:445:32:445:37 | source |
686+
| tst.js:445:32:445:37 | source |
672687
| typeahead.js:20:13:20:45 | target |
673688
| typeahead.js:20:22:20:45 | documen ... .search |
674689
| typeahead.js:20:22:20:45 | documen ... .search |
@@ -1312,6 +1327,20 @@ edges
13121327
| tst.js:428:16:428:39 | documen ... .search | tst.js:428:7:428:39 | target |
13131328
| tst.js:430:18:430:23 | target | tst.js:430:18:430:89 | target. ... data>') |
13141329
| tst.js:430:18:430:23 | target | tst.js:430:18:430:89 | target. ... data>') |
1330+
| tst.js:436:6:436:38 | source | tst.js:440:28:440:33 | source |
1331+
| tst.js:436:6:436:38 | source | tst.js:440:28:440:33 | source |
1332+
| tst.js:436:6:436:38 | source | tst.js:441:33:441:38 | source |
1333+
| tst.js:436:6:436:38 | source | tst.js:441:33:441:38 | source |
1334+
| tst.js:436:6:436:38 | source | tst.js:442:34:442:39 | source |
1335+
| tst.js:436:6:436:38 | source | tst.js:442:34:442:39 | source |
1336+
| tst.js:436:6:436:38 | source | tst.js:443:41:443:46 | source |
1337+
| tst.js:436:6:436:38 | source | tst.js:443:41:443:46 | source |
1338+
| tst.js:436:6:436:38 | source | tst.js:444:44:444:49 | source |
1339+
| tst.js:436:6:436:38 | source | tst.js:444:44:444:49 | source |
1340+
| tst.js:436:6:436:38 | source | tst.js:445:32:445:37 | source |
1341+
| tst.js:436:6:436:38 | source | tst.js:445:32:445:37 | source |
1342+
| tst.js:436:15:436:38 | documen ... .search | tst.js:436:6:436:38 | source |
1343+
| tst.js:436:15:436:38 | documen ... .search | tst.js:436:6:436:38 | source |
13151344
| typeahead.js:20:13:20:45 | target | typeahead.js:21:12:21:17 | target |
13161345
| typeahead.js:20:22:20:45 | documen ... .search | typeahead.js:20:13:20:45 | target |
13171346
| typeahead.js:20:22:20:45 | documen ... .search | typeahead.js:20:13:20:45 | target |
@@ -1531,6 +1560,12 @@ edges
15311560
| tst.js:421:20:421:27 | match[1] | tst.js:419:15:419:34 | window.location.hash | tst.js:421:20:421:27 | match[1] | Cross-site scripting vulnerability due to $@. | tst.js:419:15:419:34 | window.location.hash | user-provided value |
15321561
| tst.js:424:18:424:51 | window. ... '#')[1] | tst.js:424:18:424:37 | window.location.hash | tst.js:424:18:424:51 | window. ... '#')[1] | Cross-site scripting vulnerability due to $@. | tst.js:424:18:424:37 | window.location.hash | user-provided value |
15331562
| tst.js:430:18:430:89 | target. ... data>') | tst.js:428:16:428:39 | documen ... .search | tst.js:430:18:430:89 | target. ... data>') | Cross-site scripting vulnerability due to $@. | tst.js:428:16:428:39 | documen ... .search | user-provided value |
1563+
| tst.js:440:28:440:33 | source | tst.js:436:15:436:38 | documen ... .search | tst.js:440:28:440:33 | source | Cross-site scripting vulnerability due to $@. | tst.js:436:15:436:38 | documen ... .search | user-provided value |
1564+
| tst.js:441:33:441:38 | source | tst.js:436:15:436:38 | documen ... .search | tst.js:441:33:441:38 | source | Cross-site scripting vulnerability due to $@. | tst.js:436:15:436:38 | documen ... .search | user-provided value |
1565+
| tst.js:442:34:442:39 | source | tst.js:436:15:436:38 | documen ... .search | tst.js:442:34:442:39 | source | Cross-site scripting vulnerability due to $@. | tst.js:436:15:436:38 | documen ... .search | user-provided value |
1566+
| tst.js:443:41:443:46 | source | tst.js:436:15:436:38 | documen ... .search | tst.js:443:41:443:46 | source | Cross-site scripting vulnerability due to $@. | tst.js:436:15:436:38 | documen ... .search | user-provided value |
1567+
| tst.js:444:44:444:49 | source | tst.js:436:15:436:38 | documen ... .search | tst.js:444:44:444:49 | source | Cross-site scripting vulnerability due to $@. | tst.js:436:15:436:38 | documen ... .search | user-provided value |
1568+
| tst.js:445:32:445:37 | source | tst.js:436:15:436:38 | documen ... .search | tst.js:445:32:445:37 | source | Cross-site scripting vulnerability due to $@. | tst.js:436:15:436:38 | documen ... .search | user-provided value |
15341569
| typeahead.js:25:18:25:20 | val | typeahead.js:20:22:20:45 | documen ... .search | typeahead.js:25:18:25:20 | val | Cross-site scripting vulnerability due to $@. | typeahead.js:20:22:20:45 | documen ... .search | user-provided value |
15351570
| v-html.vue:2:8:2:23 | v-html=tainted | v-html.vue:6:42:6:58 | document.location | v-html.vue:2:8:2:23 | v-html=tainted | Cross-site scripting vulnerability due to $@. | v-html.vue:6:42:6:58 | document.location | user-provided value |
15361571
| various-concat-obfuscations.js:4:4:4:31 | "<div>" ... </div>" | various-concat-obfuscations.js:2:16:2:39 | documen ... .search | various-concat-obfuscations.js:4:4:4:31 | "<div>" ... </div>" | Cross-site scripting vulnerability due to $@. | various-concat-obfuscations.js:2:16:2:39 | documen ... .search | user-provided value |

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,21 @@ nodes
676676
| tst.js:430:18:430:23 | target |
677677
| tst.js:430:18:430:89 | target. ... data>') |
678678
| tst.js:430:18:430:89 | target. ... data>') |
679+
| tst.js:436:6:436:38 | source |
680+
| tst.js:436:15:436:38 | documen ... .search |
681+
| tst.js:436:15:436:38 | documen ... .search |
682+
| tst.js:440:28:440:33 | source |
683+
| tst.js:440:28:440:33 | source |
684+
| tst.js:441:33:441:38 | source |
685+
| tst.js:441:33:441:38 | source |
686+
| tst.js:442:34:442:39 | source |
687+
| tst.js:442:34:442:39 | source |
688+
| tst.js:443:41:443:46 | source |
689+
| tst.js:443:41:443:46 | source |
690+
| tst.js:444:44:444:49 | source |
691+
| tst.js:444:44:444:49 | source |
692+
| tst.js:445:32:445:37 | source |
693+
| tst.js:445:32:445:37 | source |
679694
| typeahead.js:9:28:9:30 | loc |
680695
| typeahead.js:9:28:9:30 | loc |
681696
| typeahead.js:10:16:10:18 | loc |
@@ -1336,6 +1351,20 @@ edges
13361351
| tst.js:428:16:428:39 | documen ... .search | tst.js:428:7:428:39 | target |
13371352
| tst.js:430:18:430:23 | target | tst.js:430:18:430:89 | target. ... data>') |
13381353
| tst.js:430:18:430:23 | target | tst.js:430:18:430:89 | target. ... data>') |
1354+
| tst.js:436:6:436:38 | source | tst.js:440:28:440:33 | source |
1355+
| tst.js:436:6:436:38 | source | tst.js:440:28:440:33 | source |
1356+
| tst.js:436:6:436:38 | source | tst.js:441:33:441:38 | source |
1357+
| tst.js:436:6:436:38 | source | tst.js:441:33:441:38 | source |
1358+
| tst.js:436:6:436:38 | source | tst.js:442:34:442:39 | source |
1359+
| tst.js:436:6:436:38 | source | tst.js:442:34:442:39 | source |
1360+
| tst.js:436:6:436:38 | source | tst.js:443:41:443:46 | source |
1361+
| tst.js:436:6:436:38 | source | tst.js:443:41:443:46 | source |
1362+
| tst.js:436:6:436:38 | source | tst.js:444:44:444:49 | source |
1363+
| tst.js:436:6:436:38 | source | tst.js:444:44:444:49 | source |
1364+
| tst.js:436:6:436:38 | source | tst.js:445:32:445:37 | source |
1365+
| tst.js:436:6:436:38 | source | tst.js:445:32:445:37 | source |
1366+
| tst.js:436:15:436:38 | documen ... .search | tst.js:436:6:436:38 | source |
1367+
| tst.js:436:15:436:38 | documen ... .search | tst.js:436:6:436:38 | source |
13391368
| typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc |
13401369
| typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc |
13411370
| typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc |

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,4 +430,17 @@ function nonGlobalSanitizer() {
430430
$("#foo").html(target.replace(/<metadata>[\s\S]*<\/metadata>/, '<metadata></metadata>')); // NOT OK
431431

432432
$("#foo").html(target.replace(/<|>/g, '')); // OK
433-
}
433+
}
434+
435+
function mootools(){
436+
var source = document.location.search;
437+
438+
new Element("div"); // OK
439+
new Element("div", {text: source}); // OK
440+
new Element("div", {html: source}); // NOT OK
441+
new Element("div").set("html", source); // NOT OK
442+
new Element("div").set({"html": source}); // NOT OK
443+
new Element("div").setProperty("html", source); // NOT OK
444+
new Element("div").setProperties({"html": source}); // NOT OK
445+
new Element("div").appendHtml(source); // NOT OK
446+
}

0 commit comments

Comments
 (0)