Skip to content

Commit 707b0f3

Browse files
asgerfasgerf
committed
JS: Use in ContainsHTMLGuard
1 parent fa1a6ee commit 707b0f3

File tree

3 files changed

+73
-8
lines changed

3 files changed

+73
-8
lines changed

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,18 +78,16 @@ module Shared {
7878
* A sanitizer guard that checks for the existence of HTML chars in a string.
7979
* E.g. `/["'&<>]/.exec(str)`.
8080
*/
81-
class ContainsHTMLGuard extends SanitizerGuard, DataFlow::MethodCallNode {
82-
DataFlow::RegExpCreationNode regExp;
83-
81+
class ContainsHTMLGuard extends SanitizerGuard, StringOps::RegExpTest {
8482
ContainsHTMLGuard() {
85-
this.getMethodName() = ["test", "exec"] and
86-
this.getReceiver().getALocalSource() = regExp and
87-
regExp.getRoot() instanceof RegExpCharacterClass and
88-
forall(string s | s = ["\"", "&", "<", ">"] | regExp.getRoot().getAMatchedString() = s)
83+
exists(RegExpCharacterClass regExp |
84+
regExp = getRegExp() and
85+
forall(string s | s = ["\"", "&", "<", ">"] | regExp.getAMatchedString() = s)
86+
)
8987
}
9088

9189
override predicate sanitizes(boolean outcome, Expr e) {
92-
outcome = false and e = this.getArgument(0).asExpr()
90+
outcome = getPolarity().booleanNot() and e = this.getStringOperand().asExpr()
9391
}
9492
}
9593

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,24 @@ nodes
8484
| react-native.js:8:18:8:24 | tainted |
8585
| react-native.js:9:27:9:33 | tainted |
8686
| react-native.js:9:27:9:33 | tainted |
87+
| sanitiser.js:20:7:20:27 | tainted |
88+
| sanitiser.js:20:17:20:27 | window.name |
89+
| sanitiser.js:20:17:20:27 | window.name |
90+
| sanitiser.js:27:21:27:44 | '<b>' + ... '</b>' |
91+
| sanitiser.js:27:21:27:44 | '<b>' + ... '</b>' |
92+
| sanitiser.js:27:29:27:35 | tainted |
93+
| sanitiser.js:34:21:34:44 | '<b>' + ... '</b>' |
94+
| sanitiser.js:34:21:34:44 | '<b>' + ... '</b>' |
95+
| sanitiser.js:34:29:34:35 | tainted |
96+
| sanitiser.js:37:21:37:44 | '<b>' + ... '</b>' |
97+
| sanitiser.js:37:21:37:44 | '<b>' + ... '</b>' |
98+
| sanitiser.js:37:29:37:35 | tainted |
99+
| sanitiser.js:42:21:42:44 | '<b>' + ... '</b>' |
100+
| sanitiser.js:42:21:42:44 | '<b>' + ... '</b>' |
101+
| sanitiser.js:42:29:42:35 | tainted |
102+
| sanitiser.js:49:21:49:44 | '<b>' + ... '</b>' |
103+
| sanitiser.js:49:21:49:44 | '<b>' + ... '</b>' |
104+
| sanitiser.js:49:29:49:35 | tainted |
87105
| stored-xss.js:2:39:2:55 | document.location |
88106
| stored-xss.js:2:39:2:55 | document.location |
89107
| stored-xss.js:2:39:2:62 | documen ... .search |
@@ -514,6 +532,23 @@ edges
514532
| react-native.js:7:7:7:33 | tainted | react-native.js:9:27:9:33 | tainted |
515533
| react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted |
516534
| react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted |
535+
| sanitiser.js:20:7:20:27 | tainted | sanitiser.js:27:29:27:35 | tainted |
536+
| sanitiser.js:20:7:20:27 | tainted | sanitiser.js:34:29:34:35 | tainted |
537+
| sanitiser.js:20:7:20:27 | tainted | sanitiser.js:37:29:37:35 | tainted |
538+
| sanitiser.js:20:7:20:27 | tainted | sanitiser.js:42:29:42:35 | tainted |
539+
| sanitiser.js:20:7:20:27 | tainted | sanitiser.js:49:29:49:35 | tainted |
540+
| sanitiser.js:20:17:20:27 | window.name | sanitiser.js:20:7:20:27 | tainted |
541+
| sanitiser.js:20:17:20:27 | window.name | sanitiser.js:20:7:20:27 | tainted |
542+
| sanitiser.js:27:29:27:35 | tainted | sanitiser.js:27:21:27:44 | '<b>' + ... '</b>' |
543+
| sanitiser.js:27:29:27:35 | tainted | sanitiser.js:27:21:27:44 | '<b>' + ... '</b>' |
544+
| sanitiser.js:34:29:34:35 | tainted | sanitiser.js:34:21:34:44 | '<b>' + ... '</b>' |
545+
| sanitiser.js:34:29:34:35 | tainted | sanitiser.js:34:21:34:44 | '<b>' + ... '</b>' |
546+
| sanitiser.js:37:29:37:35 | tainted | sanitiser.js:37:21:37:44 | '<b>' + ... '</b>' |
547+
| sanitiser.js:37:29:37:35 | tainted | sanitiser.js:37:21:37:44 | '<b>' + ... '</b>' |
548+
| sanitiser.js:42:29:42:35 | tainted | sanitiser.js:42:21:42:44 | '<b>' + ... '</b>' |
549+
| sanitiser.js:42:29:42:35 | tainted | sanitiser.js:42:21:42:44 | '<b>' + ... '</b>' |
550+
| sanitiser.js:49:29:49:35 | tainted | sanitiser.js:49:21:49:44 | '<b>' + ... '</b>' |
551+
| sanitiser.js:49:29:49:35 | tainted | sanitiser.js:49:21:49:44 | '<b>' + ... '</b>' |
517552
| stored-xss.js:2:39:2:55 | document.location | stored-xss.js:2:39:2:62 | documen ... .search |
518553
| stored-xss.js:2:39:2:55 | document.location | stored-xss.js:2:39:2:62 | documen ... .search |
519554
| stored-xss.js:2:39:2:62 | documen ... .search | stored-xss.js:5:20:5:52 | session ... ssion') |
@@ -834,6 +869,11 @@ edges
834869
| optionalSanitizer.js:45:18:45:56 | sanitiz ... target | optionalSanitizer.js:26:16:26:32 | document.location | optionalSanitizer.js:45:18:45:56 | sanitiz ... target | Cross-site scripting vulnerability due to $@. | optionalSanitizer.js:26:16:26:32 | document.location | user-provided value |
835870
| react-native.js:8:18:8:24 | tainted | react-native.js:7:17:7:33 | req.param("code") | react-native.js:8:18:8:24 | tainted | Cross-site scripting vulnerability due to $@. | react-native.js:7:17:7:33 | req.param("code") | user-provided value |
836871
| react-native.js:9:27:9:33 | tainted | react-native.js:7:17:7:33 | req.param("code") | react-native.js:9:27:9:33 | tainted | Cross-site scripting vulnerability due to $@. | react-native.js:7:17:7:33 | req.param("code") | user-provided value |
872+
| sanitiser.js:27:21:27:44 | '<b>' + ... '</b>' | sanitiser.js:20:17:20:27 | window.name | sanitiser.js:27:21:27:44 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | sanitiser.js:20:17:20:27 | window.name | user-provided value |
873+
| sanitiser.js:34:21:34:44 | '<b>' + ... '</b>' | sanitiser.js:20:17:20:27 | window.name | sanitiser.js:34:21:34:44 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | sanitiser.js:20:17:20:27 | window.name | user-provided value |
874+
| sanitiser.js:37:21:37:44 | '<b>' + ... '</b>' | sanitiser.js:20:17:20:27 | window.name | sanitiser.js:37:21:37:44 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | sanitiser.js:20:17:20:27 | window.name | user-provided value |
875+
| sanitiser.js:42:21:42:44 | '<b>' + ... '</b>' | sanitiser.js:20:17:20:27 | window.name | sanitiser.js:42:21:42:44 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | sanitiser.js:20:17:20:27 | window.name | user-provided value |
876+
| sanitiser.js:49:21:49:44 | '<b>' + ... '</b>' | sanitiser.js:20:17:20:27 | window.name | sanitiser.js:49:21:49:44 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | sanitiser.js:20:17:20:27 | window.name | user-provided value |
837877
| stored-xss.js:5:20:5:52 | session ... ssion') | stored-xss.js:2:39:2:55 | document.location | stored-xss.js:5:20:5:52 | session ... ssion') | Cross-site scripting vulnerability due to $@. | stored-xss.js:2:39:2:55 | document.location | user-provided value |
838878
| stored-xss.js:8:20:8:48 | localSt ... local') | stored-xss.js:3:35:3:51 | document.location | stored-xss.js:8:20:8:48 | localSt ... local') | Cross-site scripting vulnerability due to $@. | stored-xss.js:3:35:3:51 | document.location | user-provided value |
839879
| stored-xss.js:12:20:12:54 | "<a hre ... ar</a>" | stored-xss.js:3:35:3:51 | document.location | stored-xss.js:12:20:12:54 | "<a hre ... ar</a>" | Cross-site scripting vulnerability due to $@. | stored-xss.js:3:35:3:51 | document.location | user-provided value |

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,31 @@ function test() {
1717
var elt = document.createElement();
1818
elt.innerHTML = "<a href=\"" + escapeAttr(tainted) + "\">" + escapeHtml(tainted) + "</a>"; // OK
1919
elt.innerHTML = "<div>" + escapeAttr(tainted) + "</div>"; // NOT OK, but not flagged
20+
21+
const regex = /[<>'"&]/;
22+
if (regex.test(tainted)) {
23+
elt.innerHTML = '<b>' + tainted + '</b>'; // NOT OK
24+
} else {
25+
elt.innerHTML = '<b>' + tainted + '</b>'; // OK
26+
}
27+
if (!regex.test(tainted)) {
28+
elt.innerHTML = '<b>' + tainted + '</b>'; // OK
29+
} else {
30+
elt.innerHTML = '<b>' + tainted + '</b>'; // NOT OK
31+
}
32+
if (regex.exec(tainted)) {
33+
elt.innerHTML = '<b>' + tainted + '</b>'; // NOT OK
34+
} else {
35+
elt.innerHTML = '<b>' + tainted + '</b>'; // OK
36+
}
37+
if (regex.exec(tainted) != null) {
38+
elt.innerHTML = '<b>' + tainted + '</b>'; // NOT OK
39+
} else {
40+
elt.innerHTML = '<b>' + tainted + '</b>'; // OK
41+
}
42+
if (regex.exec(tainted) == null) {
43+
elt.innerHTML = '<b>' + tainted + '</b>'; // OK
44+
} else {
45+
elt.innerHTML = '<b>' + tainted + '</b>'; // NOT OK
46+
}
2047
}

0 commit comments

Comments
 (0)