Skip to content

Commit 3397533

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request #628 from xiemaisi/js/setUnsafeHTML
Approved by esben-semmle
2 parents 0a496c1 + ef347b3 commit 3397533

File tree

6 files changed

+38
-2
lines changed

6 files changed

+38
-2
lines changed

‎change-notes/1.20/analysis-javascript.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
| **Query** | **Expected impact** | **Change** |
2020
|--------------------------------------------|------------------------------|------------------------------------------------------------------------------|
21+
| Client-side cross-site scripting | More results | This rule now recognizes WinJS functions that are vulnerable to HTML injection. |
2122
| Unused variable, import, function or class | Fewer false-positive results | This rule now flags fewer variables that are implictly used by JSX elements. |
22-
| | | |
2323

2424
## Changes to QL libraries

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* @name Client side cross-site scripting
2+
* @name Client-side cross-site scripting
33
* @description Writing user input directly to the DOM allows for
44
* a cross-site scripting vulnerability.
55
* @kind path-problem

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,13 @@ module DomBasedXss {
9696
or
9797
// call to an Angular method that interprets its argument as HTML
9898
any(AngularJS::AngularJSCall call).interpretsArgumentAsHtml(this.asExpr())
99+
or
100+
// call to a WinJS function that interprets its argument as HTML
101+
exists (DataFlow::MethodCallNode mcn, string m |
102+
m = "setInnerHTMLUnsafe" or m = "setOuterHTMLUnsafe" |
103+
mcn.getMethodName() = m and
104+
this = mcn.getArgument(1)
105+
)
99106
}
100107
}
101108

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,12 @@ nodes
205205
| tst.js:244:39:244:55 | props.propTainted |
206206
| tst.js:248:60:248:82 | this.st ... Tainted |
207207
| tst.js:252:23:252:29 | tainted |
208+
| winjs.js:2:7:2:53 | tainted |
209+
| winjs.js:2:17:2:33 | document.location |
210+
| winjs.js:2:17:2:40 | documen ... .search |
211+
| winjs.js:2:17:2:53 | documen ... ring(1) |
212+
| winjs.js:3:43:3:49 | tainted |
213+
| winjs.js:4:43:4:49 | tainted |
208214
| xss-through-filenames.js:7:43:7:48 | files1 |
209215
| xss-through-filenames.js:8:18:8:23 | files1 |
210216
| xss-through-filenames.js:25:43:25:48 | files1 |
@@ -377,6 +383,11 @@ edges
377383
| tst.js:238:23:238:29 | tainted | tst.js:228:32:228:49 | prevProps.tainted4 |
378384
| tst.js:244:39:244:55 | props.propTainted | tst.js:248:60:248:82 | this.st ... Tainted |
379385
| tst.js:252:23:252:29 | tainted | tst.js:244:39:244:55 | props.propTainted |
386+
| winjs.js:2:7:2:53 | tainted | winjs.js:3:43:3:49 | tainted |
387+
| winjs.js:2:7:2:53 | tainted | winjs.js:4:43:4:49 | tainted |
388+
| winjs.js:2:17:2:33 | document.location | winjs.js:2:17:2:40 | documen ... .search |
389+
| winjs.js:2:17:2:40 | documen ... .search | winjs.js:2:17:2:53 | documen ... ring(1) |
390+
| winjs.js:2:17:2:53 | documen ... ring(1) | winjs.js:2:7:2:53 | tainted |
380391
| xss-through-filenames.js:7:43:7:48 | files1 | xss-through-filenames.js:8:18:8:23 | files1 |
381392
| xss-through-filenames.js:25:43:25:48 | files1 | xss-through-filenames.js:26:19:26:24 | files1 |
382393
| xss-through-filenames.js:25:43:25:48 | files1 | xss-through-filenames.js:30:34:30:37 | file |

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,12 @@ nodes
162162
| tst.js:244:39:244:55 | props.propTainted |
163163
| tst.js:248:60:248:82 | this.st ... Tainted |
164164
| tst.js:252:23:252:29 | tainted |
165+
| winjs.js:2:7:2:53 | tainted |
166+
| winjs.js:2:17:2:33 | document.location |
167+
| winjs.js:2:17:2:40 | documen ... .search |
168+
| winjs.js:2:17:2:53 | documen ... ring(1) |
169+
| winjs.js:3:43:3:49 | tainted |
170+
| winjs.js:4:43:4:49 | tainted |
165171
edges
166172
| addEventListener.js:1:43:1:47 | event | addEventListener.js:2:20:2:24 | event |
167173
| addEventListener.js:2:20:2:24 | event | addEventListener.js:2:20:2:29 | event.data |
@@ -288,6 +294,11 @@ edges
288294
| tst.js:238:23:238:29 | tainted | tst.js:228:32:228:49 | prevProps.tainted4 |
289295
| tst.js:244:39:244:55 | props.propTainted | tst.js:248:60:248:82 | this.st ... Tainted |
290296
| tst.js:252:23:252:29 | tainted | tst.js:244:39:244:55 | props.propTainted |
297+
| winjs.js:2:7:2:53 | tainted | winjs.js:3:43:3:49 | tainted |
298+
| winjs.js:2:7:2:53 | tainted | winjs.js:4:43:4:49 | tainted |
299+
| winjs.js:2:17:2:33 | document.location | winjs.js:2:17:2:40 | documen ... .search |
300+
| winjs.js:2:17:2:40 | documen ... .search | winjs.js:2:17:2:53 | documen ... ring(1) |
301+
| winjs.js:2:17:2:53 | documen ... ring(1) | winjs.js:2:7:2:53 | tainted |
291302
#select
292303
| addEventListener.js:2:20:2:29 | event.data | addEventListener.js:1:43:1:47 | event | addEventListener.js:2:20:2:29 | event.data | Cross-site scripting vulnerability due to $@. | addEventListener.js:1:43:1:47 | event | user-provided value |
293304
| jquery.js:4:5:4:11 | tainted | jquery.js:2:17:2:33 | document.location | jquery.js:4:5:4:11 | tainted | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:33 | document.location | user-provided value |
@@ -349,3 +360,5 @@ edges
349360
| tst.js:224:28:224:46 | this.props.tainted3 | tst.js:194:19:194:35 | document.location | tst.js:224:28:224:46 | this.props.tainted3 | Cross-site scripting vulnerability due to $@. | tst.js:194:19:194:35 | document.location | user-provided value |
350361
| tst.js:228:32:228:49 | prevProps.tainted4 | tst.js:194:19:194:35 | document.location | tst.js:228:32:228:49 | prevProps.tainted4 | Cross-site scripting vulnerability due to $@. | tst.js:194:19:194:35 | document.location | user-provided value |
351362
| tst.js:248:60:248:82 | this.st ... Tainted | tst.js:194:19:194:35 | document.location | tst.js:248:60:248:82 | this.st ... Tainted | Cross-site scripting vulnerability due to $@. | tst.js:194:19:194:35 | document.location | user-provided value |
363+
| winjs.js:3:43:3:49 | tainted | winjs.js:2:17:2:33 | document.location | winjs.js:3:43:3:49 | tainted | Cross-site scripting vulnerability due to $@. | winjs.js:2:17:2:33 | document.location | user-provided value |
364+
| winjs.js:4:43:4:49 | tainted | winjs.js:2:17:2:33 | document.location | winjs.js:4:43:4:49 | tainted | Cross-site scripting vulnerability due to $@. | winjs.js:2:17:2:33 | document.location | user-provided value |

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
function test(elt) {
2+
var tainted = document.location.search.substring(1);
3+
WinJS.Utilities.setInnerHTMLUnsafe(elt, tainted);
4+
WinJS.Utilities.setOuterHTMLUnsafe(elt, tainted);
5+
}

0 commit comments

Comments
 (0)