Skip to content

Commit 101d435

Browse files
committed
detect DOM nodes from event callbacks
1 parent be96364 commit 101d435

File tree

3 files changed

+30
-2
lines changed

3 files changed

+30
-2
lines changed

javascript/ql/src/semmle/javascript/DOM.qll

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,9 +361,19 @@ module DOM {
361361
* Gets a reference to a DOM event.
362362
*/
363363
private DataFlow::SourceNode domEventSource() {
364+
// e.g. <form onSubmit={e => e.target}/>
364365
exists(JSXAttribute attr | attr.getName().matches("on%") |
365366
result = attr.getValue().flow().getABoundFunctionValue(0).getParameter(0)
366367
)
368+
or
369+
// node.addEventListener("submit", e => e.target)
370+
result = domValueRef().getAMethodCall("addEventListener").getABoundCallbackParameter(1, 0)
371+
or
372+
// node.onSubmit = (e => e.target);
373+
exists(DataFlow::PropWrite write | write = domValueRef().getAPropertyWrite() |
374+
write.getPropertyName().matches("on%") and
375+
result = write.getRhs().getAFunctionValue().getParameter(0)
376+
)
367377
}
368378

369379
/** Gets a data flow node that refers directly to a value from the DOM. */
@@ -377,7 +387,6 @@ module DOM {
377387
t.start() and
378388
result = domValueRef().getAMethodCall(["item", "namedItem"])
379389
or
380-
// e.g. <form onSubmit={e => e.target}/>
381390
t.startInProp("target") and
382391
result = domEventSource()
383392
or

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ nodes
4444
| forms.js:93:25:93:30 | values |
4545
| forms.js:93:25:93:35 | values.name |
4646
| forms.js:93:25:93:35 | values.name |
47+
| forms.js:103:23:103:36 | e.target.value |
48+
| forms.js:103:23:103:36 | e.target.value |
49+
| forms.js:103:23:103:36 | e.target.value |
50+
| forms.js:107:23:107:36 | e.target.value |
51+
| forms.js:107:23:107:36 | e.target.value |
52+
| forms.js:107:23:107:36 | e.target.value |
4753
| xss-through-dom.js:2:16:2:34 | $("textarea").val() |
4854
| xss-through-dom.js:2:16:2:34 | $("textarea").val() |
4955
| xss-through-dom.js:2:16:2:34 | $("textarea").val() |
@@ -130,6 +136,8 @@ edges
130136
| forms.js:92:26:92:36 | getValues() | forms.js:92:17:92:36 | values |
131137
| forms.js:93:25:93:30 | values | forms.js:93:25:93:35 | values.name |
132138
| forms.js:93:25:93:30 | values | forms.js:93:25:93:35 | values.name |
139+
| forms.js:103:23:103:36 | e.target.value | forms.js:103:23:103:36 | e.target.value |
140+
| forms.js:107:23:107:36 | e.target.value | forms.js:107:23:107:36 | e.target.value |
133141
| xss-through-dom.js:2:16:2:34 | $("textarea").val() | xss-through-dom.js:2:16:2:34 | $("textarea").val() |
134142
| xss-through-dom.js:4:16:4:40 | $(".som ... .text() | xss-through-dom.js:4:16:4:40 | $(".som ... .text() |
135143
| xss-through-dom.js:8:16:8:53 | $(".som ... arget") | xss-through-dom.js:8:16:8:53 | $(".som ... arget") |
@@ -159,6 +167,8 @@ edges
159167
| forms.js:57:19:57:32 | e.target.value | forms.js:57:19:57:32 | e.target.value | forms.js:57:19:57:32 | e.target.value | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:57:19:57:32 | e.target.value | DOM text |
160168
| forms.js:72:19:72:27 | data.name | forms.js:71:21:71:24 | data | forms.js:72:19:72:27 | data.name | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:71:21:71:24 | data | DOM text |
161169
| forms.js:93:25:93:35 | values.name | forms.js:92:26:92:36 | getValues() | forms.js:93:25:93:35 | values.name | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:92:26:92:36 | getValues() | DOM text |
170+
| forms.js:103:23:103:36 | e.target.value | forms.js:103:23:103:36 | e.target.value | forms.js:103:23:103:36 | e.target.value | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:103:23:103:36 | e.target.value | DOM text |
171+
| forms.js:107:23:107:36 | e.target.value | forms.js:107:23:107:36 | e.target.value | forms.js:107:23:107:36 | e.target.value | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:107:23:107:36 | e.target.value | DOM text |
162172
| 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() | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:2:16:2:34 | $("textarea").val() | DOM text |
163173
| 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() | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:4:16:4:40 | $(".som ... .text() | DOM text |
164174
| 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") | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:8:16:8:53 | $(".som ... arget") | DOM text |

javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/forms.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,13 @@ function HookForm2() {
9797
</form>
9898
);
9999
}
100-
100+
101+
function vanillaJS() {
102+
document.querySelector("form.myform").addEventListener("submit", e => {
103+
$("#id").html(e.target.value); // NOT OK
104+
});
105+
106+
document.querySelector("form.myform").onsubmit = function (e) {
107+
$("#id").html(e.target.value); // NOT OK
108+
}
109+
}

0 commit comments

Comments
 (0)