Skip to content

Commit 14b551f

Browse files
erik-krogherik-krogh
committed
Xss through DOM
1 parent 55edfed commit 14b551f

6 files changed

Lines changed: 294 additions & 0 deletions

File tree

‎javascript/ql/src/Security/CWE-079/XssThroughDom.ql‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Cross-site scripting through DOM
3+
* @description Writing user controlled DOM to HTML can allow for
4+
* a cross-site scripting vulnerability.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision medium
8+
* @id js/xss-through-dom
9+
* @tags security
10+
* external/cwe/cwe-079
11+
* external/cwe/cwe-116
12+
*/
13+
14+
import javascript
15+
import semmle.javascript.security.dataflow.XssThroughDom::XssThroughDom
16+
import DataFlow::PathGraph
17+
18+
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
19+
where cfg.hasFlowPath(source, sink)
20+
select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to $@.",
21+
source.getNode(), "DOM text"

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,3 +413,9 @@ module StoredXss {
413413

414414
private class UriEncodingSanitizer extends Sanitizer, Shared::UriEncodingSanitizer { }
415415
}
416+
417+
/** Provides classes and predicates for the XSS through DOM query. */
418+
module XssThroughDom {
419+
/** A data flow source for XSS through DOM vulnerabilities. */
420+
abstract class Source extends Shared::Source { }
421+
}

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

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about
3+
* cross-site scripting vulnerabilities through the DOM.
4+
*/
5+
6+
import javascript
7+
8+
/**
9+
* Classes and predicates for the XSS through DOM query.
10+
*/
11+
module XssThroughDom {
12+
import Xss::XssThroughDom
13+
private import semmle.javascript.security.dataflow.Xss::DomBasedXss as DomBasedXss
14+
private import semmle.javascript.dataflow.InferredTypes
15+
16+
/**
17+
* A taint-tracking configuration for reasoning about XSS through the DOM.
18+
*/
19+
class Configuration extends TaintTracking::Configuration {
20+
Configuration() { this = "XssThroughDOM" }
21+
22+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
23+
24+
override predicate isSink(DataFlow::Node sink) { sink instanceof DomBasedXss::Sink }
25+
26+
override predicate isSanitizer(DataFlow::Node node) {
27+
super.isSanitizer(node) or
28+
node instanceof DomBasedXss::Sanitizer
29+
}
30+
31+
override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
32+
guard instanceof TypeTestGuard or
33+
guard instanceof HasNodePropertySanitizerGuard
34+
}
35+
}
36+
37+
/**
38+
* Gets an attribute name that could store user controlled data.
39+
*
40+
* Attributes such as "id", "href", and "src" are often used as input to HTML.
41+
* However, they are either rarely controlable by a user, or already a sink for other XSS vulnerabilities.
42+
* Such attributes are therefore ignored.
43+
*/
44+
bindingset[result]
45+
string unsafeAttributeName() {
46+
result.regexpMatch("data-.*") or
47+
result = ["name", "value"]
48+
}
49+
50+
/**
51+
* A source for text from the DOM from a JQuery method call.
52+
*/
53+
class JQueryTextSource extends Source, JQuery::MethodCall {
54+
JQueryTextSource() {
55+
(
56+
this.getMethodName() = ["text", "val"] and this.getNumArgument() = 0
57+
or
58+
this.getMethodName() = "attr" and
59+
this.getNumArgument() = 1 and
60+
forex(InferredType t | t = this.getArgument(0).analyze().getAType() | t = TTString()) and
61+
this.getArgument(0).mayHaveStringValue(unsafeAttributeName())
62+
) and
63+
// looks like a $("<p>" + ... ) source, which is benign for this query.
64+
not this
65+
.getReceiver()
66+
.(DataFlow::CallNode)
67+
.getAnArgument()
68+
.(StringOps::ConcatenationRoot)
69+
.getConstantStringParts()
70+
.substring(0, 1) = "<"
71+
}
72+
}
73+
74+
/**
75+
* A source for text from the DOM from a DOM property read or call to `getAttribute()`.
76+
*/
77+
class DOMTextSource extends Source {
78+
DOMTextSource() {
79+
exists(DataFlow::PropRead read | read = this |
80+
read.getBase().getALocalSource() = DOM::domValueRef() and
81+
exists(string propName | propName = ["innerText", "textContent", "value", "name"] |
82+
read.getPropertyName() = propName or
83+
read.getPropertyNameExpr().flow().mayHaveStringValue(propName)
84+
)
85+
)
86+
or
87+
exists(DataFlow::MethodCallNode mcn | mcn = this |
88+
mcn.getReceiver().getALocalSource() = DOM::domValueRef() and
89+
mcn.getMethodName() = "getAttribute" and
90+
mcn.getArgument(0).mayHaveStringValue(unsafeAttributeName())
91+
)
92+
}
93+
}
94+
95+
/**
96+
* A test of form `typeof x === "something"`, preventing `x` from being a string in some cases.
97+
*
98+
* This sanitizer helps prune infeasible paths in type-overloaded functions.
99+
*/
100+
class TypeTestGuard extends TaintTracking::SanitizerGuardNode, DataFlow::ValueNode {
101+
override EqualityTest astNode;
102+
TypeofExpr typeof;
103+
boolean polarity;
104+
105+
TypeTestGuard() {
106+
astNode.getAnOperand() = typeof and
107+
(
108+
// typeof x === "string" sanitizes `x` when it evaluates to false
109+
astNode.getAnOperand().getStringValue() = "string" and
110+
polarity = astNode.getPolarity().booleanNot()
111+
or
112+
// typeof x === "object" sanitizes `x` when it evaluates to true
113+
astNode.getAnOperand().getStringValue() != "string" and
114+
polarity = astNode.getPolarity()
115+
)
116+
}
117+
118+
override predicate sanitizes(boolean outcome, Expr e) {
119+
polarity = outcome and
120+
e = typeof.getOperand()
121+
}
122+
}
123+
124+
/**
125+
* The precense of a `nodeType` or `jquery` property indicates that the value is a DOM node, and not the text of a DOM node.
126+
*
127+
* This sanitizer helps prune infeasible paths in type-overloaded functions.
128+
*/
129+
class HasNodePropertySanitizerGuard extends TaintTracking::SanitizerGuardNode {
130+
DataFlow::PropRead read;
131+
132+
HasNodePropertySanitizerGuard() {
133+
read = this and
134+
read.getPropertyName() = ["nodeType", "jquery"]
135+
}
136+
137+
override predicate sanitizes(boolean outcome, Expr e) {
138+
e = read.getBase().asExpr() and outcome = true
139+
}
140+
}
141+
}

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
nodes
2+
| xss-through-dom.js:2:16:2:34 | $("textarea").val() |
3+
| xss-through-dom.js:2:16:2:34 | $("textarea").val() |
4+
| xss-through-dom.js:2:16:2:34 | $("textarea").val() |
5+
| xss-through-dom.js:4:16:4:40 | $(".som ... .text() |
6+
| xss-through-dom.js:4:16:4:40 | $(".som ... .text() |
7+
| xss-through-dom.js:4:16:4:40 | $(".som ... .text() |
8+
| xss-through-dom.js:8:16:8:53 | $(".som ... arget") |
9+
| xss-through-dom.js:8:16:8:53 | $(".som ... arget") |
10+
| xss-through-dom.js:8:16:8:53 | $(".som ... arget") |
11+
| xss-through-dom.js:11:3:11:42 | documen ... nerText |
12+
| xss-through-dom.js:11:3:11:42 | documen ... nerText |
13+
| xss-through-dom.js:11:3:11:42 | documen ... nerText |
14+
| xss-through-dom.js:19:3:19:44 | documen ... Content |
15+
| xss-through-dom.js:19:3:19:44 | documen ... Content |
16+
| xss-through-dom.js:19:3:19:44 | documen ... Content |
17+
| xss-through-dom.js:23:3:23:48 | documen ... ].value |
18+
| xss-through-dom.js:23:3:23:48 | documen ... ].value |
19+
| xss-through-dom.js:23:3:23:48 | documen ... ].value |
20+
| xss-through-dom.js:27:3:27:61 | documen ... arget') |
21+
| xss-through-dom.js:27:3:27:61 | documen ... arget') |
22+
| xss-through-dom.js:27:3:27:61 | documen ... arget') |
23+
| xss-through-dom.js:51:30:51:48 | $("textarea").val() |
24+
| xss-through-dom.js:51:30:51:48 | $("textarea").val() |
25+
| xss-through-dom.js:51:30:51:48 | $("textarea").val() |
26+
| xss-through-dom.js:54:31:54:49 | $("textarea").val() |
27+
| xss-through-dom.js:54:31:54:49 | $("textarea").val() |
28+
| xss-through-dom.js:54:31:54:49 | $("textarea").val() |
29+
| xss-through-dom.js:56:30:56:51 | $("inpu ... 0).name |
30+
| xss-through-dom.js:56:30:56:51 | $("inpu ... 0).name |
31+
| xss-through-dom.js:56:30:56:51 | $("inpu ... 0).name |
32+
| xss-through-dom.js:57:30:57:67 | $("inpu ... "name") |
33+
| xss-through-dom.js:57:30:57:67 | $("inpu ... "name") |
34+
| xss-through-dom.js:57:30:57:67 | $("inpu ... "name") |
35+
| xss-through-dom.js:61:30:61:69 | $(docum ... value") |
36+
| xss-through-dom.js:61:30:61:69 | $(docum ... value") |
37+
| xss-through-dom.js:61:30:61:69 | $(docum ... value") |
38+
edges
39+
| xss-through-dom.js:2:16:2:34 | $("textarea").val() | xss-through-dom.js:2:16:2:34 | $("textarea").val() |
40+
| xss-through-dom.js:4:16:4:40 | $(".som ... .text() | xss-through-dom.js:4:16:4:40 | $(".som ... .text() |
41+
| xss-through-dom.js:8:16:8:53 | $(".som ... arget") | xss-through-dom.js:8:16:8:53 | $(".som ... arget") |
42+
| xss-through-dom.js:11:3:11:42 | documen ... nerText | xss-through-dom.js:11:3:11:42 | documen ... nerText |
43+
| xss-through-dom.js:19:3:19:44 | documen ... Content | xss-through-dom.js:19:3:19:44 | documen ... Content |
44+
| xss-through-dom.js:23:3:23:48 | documen ... ].value | xss-through-dom.js:23:3:23:48 | documen ... ].value |
45+
| xss-through-dom.js:27:3:27:61 | documen ... arget') | xss-through-dom.js:27:3:27:61 | documen ... arget') |
46+
| xss-through-dom.js:51:30:51:48 | $("textarea").val() | xss-through-dom.js:51:30:51:48 | $("textarea").val() |
47+
| xss-through-dom.js:54:31:54:49 | $("textarea").val() | xss-through-dom.js:54:31:54:49 | $("textarea").val() |
48+
| xss-through-dom.js:56:30:56:51 | $("inpu ... 0).name | xss-through-dom.js:56:30:56:51 | $("inpu ... 0).name |
49+
| xss-through-dom.js:57:30:57:67 | $("inpu ... "name") | xss-through-dom.js:57:30:57:67 | $("inpu ... "name") |
50+
| xss-through-dom.js:61:30:61:69 | $(docum ... value") | xss-through-dom.js:61:30:61:69 | $(docum ... value") |
51+
#select
52+
| xss-through-dom.js:2:16:2:34 | $("textarea").val() | xss-through-dom.js:2:16:2:34 | $("textarea").val() | xss-through-dom.js:2:16:2:34 | $("textarea").val() | Cross-site scripting vulnerability due to $@. | xss-through-dom.js:2:16:2:34 | $("textarea").val() | DOM text |
53+
| xss-through-dom.js:4:16:4:40 | $(".som ... .text() | xss-through-dom.js:4:16:4:40 | $(".som ... .text() | xss-through-dom.js:4:16:4:40 | $(".som ... .text() | Cross-site scripting vulnerability due to $@. | xss-through-dom.js:4:16:4:40 | $(".som ... .text() | DOM text |
54+
| xss-through-dom.js:8:16:8:53 | $(".som ... arget") | xss-through-dom.js:8:16:8:53 | $(".som ... arget") | xss-through-dom.js:8:16:8:53 | $(".som ... arget") | Cross-site scripting vulnerability due to $@. | xss-through-dom.js:8:16:8:53 | $(".som ... arget") | DOM text |
55+
| xss-through-dom.js:11:3:11:42 | documen ... nerText | xss-through-dom.js:11:3:11:42 | documen ... nerText | xss-through-dom.js:11:3:11:42 | documen ... nerText | Cross-site scripting vulnerability due to $@. | xss-through-dom.js:11:3:11:42 | documen ... nerText | DOM text |
56+
| xss-through-dom.js:19:3:19:44 | documen ... Content | xss-through-dom.js:19:3:19:44 | documen ... Content | xss-through-dom.js:19:3:19:44 | documen ... Content | Cross-site scripting vulnerability due to $@. | xss-through-dom.js:19:3:19:44 | documen ... Content | DOM text |
57+
| xss-through-dom.js:23:3:23:48 | documen ... ].value | xss-through-dom.js:23:3:23:48 | documen ... ].value | xss-through-dom.js:23:3:23:48 | documen ... ].value | Cross-site scripting vulnerability due to $@. | xss-through-dom.js:23:3:23:48 | documen ... ].value | DOM text |
58+
| xss-through-dom.js:27:3:27:61 | documen ... arget') | xss-through-dom.js:27:3:27:61 | documen ... arget') | xss-through-dom.js:27:3:27:61 | documen ... arget') | Cross-site scripting vulnerability due to $@. | xss-through-dom.js:27:3:27:61 | documen ... arget') | DOM text |
59+
| xss-through-dom.js:51:30:51:48 | $("textarea").val() | xss-through-dom.js:51:30:51:48 | $("textarea").val() | xss-through-dom.js:51:30:51:48 | $("textarea").val() | Cross-site scripting vulnerability due to $@. | xss-through-dom.js:51:30:51:48 | $("textarea").val() | DOM text |
60+
| xss-through-dom.js:54:31:54:49 | $("textarea").val() | xss-through-dom.js:54:31:54:49 | $("textarea").val() | xss-through-dom.js:54:31:54:49 | $("textarea").val() | Cross-site scripting vulnerability due to $@. | xss-through-dom.js:54:31:54:49 | $("textarea").val() | DOM text |
61+
| xss-through-dom.js:56:30:56:51 | $("inpu ... 0).name | xss-through-dom.js:56:30:56:51 | $("inpu ... 0).name | xss-through-dom.js:56:30:56:51 | $("inpu ... 0).name | Cross-site scripting vulnerability due to $@. | xss-through-dom.js:56:30:56:51 | $("inpu ... 0).name | DOM text |
62+
| xss-through-dom.js:57:30:57:67 | $("inpu ... "name") | xss-through-dom.js:57:30:57:67 | $("inpu ... "name") | xss-through-dom.js:57:30:57:67 | $("inpu ... "name") | Cross-site scripting vulnerability due to $@. | xss-through-dom.js:57:30:57:67 | $("inpu ... "name") | DOM text |
63+
| xss-through-dom.js:61:30:61:69 | $(docum ... value") | xss-through-dom.js:61:30:61:69 | $(docum ... value") | xss-through-dom.js:61:30:61:69 | $(docum ... value") | Cross-site scripting vulnerability due to $@. | xss-through-dom.js:61:30:61:69 | $(docum ... value") | DOM text |

‎javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE-079/XssThroughDom.ql

‎javascript/ql/test/query-tests/Security/CWE-079/xss-through-dom.js‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
(function () {
2+
$("#id").html($("textarea").val()); // NOT OK.
3+
4+
$("#id").html($(".some-element").text()); // NOT OK.
5+
6+
$("#id").html($(".some-element").attr("foo", "bar")); // OK.
7+
$("#id").html($(".some-element").attr({"foo": "bar"})); // OK.
8+
$("#id").html($(".some-element").attr("data-target")); // NOT OK.
9+
10+
$("#id").html(
11+
document.getElementById("foo").innerText // NOT OK.
12+
);
13+
14+
$("#id").html(
15+
document.getElementById("foo").innerHTML // OK - only repeats existing XSS.
16+
);
17+
18+
$("#id").html(
19+
document.getElementById("foo").textContent // NOT OK.
20+
);
21+
22+
$("#id").html(
23+
document.querySelectorAll("textarea")[0].value // NOT OK.
24+
);
25+
26+
$("#id").html(
27+
document.getElementById('div1').getAttribute('data-target') // NOT OK
28+
);
29+
30+
function safe1(x) { // overloaded function.
31+
if (x.jquery) {
32+
var foo = $(x); // OK
33+
}
34+
35+
}
36+
safe1($("textarea").val());
37+
38+
function safe2(x) { // overloaded function.
39+
if (typeof x === "object") {
40+
var foo = $(x); // OK
41+
}
42+
}
43+
safe2($("textarea").val());
44+
45+
46+
$("#id").html(
47+
$("<p>" + something() + "</p>").text() // OK - this is for a flow-step to catch, not this query.
48+
);
49+
50+
51+
$("#id").get(0).innerHTML = $("textarea").val(); // NOT OK.
52+
53+
var base = $("#id");
54+
base[html ? 'html' : 'text']($("textarea").val()); // NOT OK.
55+
56+
$("#id").get(0).innerHTML = $("input").get(0).name; // NOT OK.
57+
$("#id").get(0).innerHTML = $("input").get(0).getAttribute("name"); // NOT OK.
58+
59+
$("#id").get(0).innerHTML = $("input").getAttribute("id"); // OK.
60+
61+
$("#id").get(0).innerHTML = $(document).find("option").attr("value"); // NOT OK.
62+
})();

0 commit comments

Comments
 (0)