JS: query and tests for unsafe HTML expansion

Author: esbena

javascript/ql/src/Security/CWE-116/UnsafeHtmlExpansion.ql

@@ -0,0 +1,67 @@
+/**
+ * @name Unsafe expansion of shorthand HTML tag
+ * @description Using regular expressions to expand shorthand HTML
+ *              tags may lead to cross-site scripting vulnerabilities.
+ * @kind problem
+ * @problem.severity warning
+ * @precision very-high
+ * @id js/unsafe-html-expansion
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-079
+ *       external/cwe/cwe-116
+ */
+
+import javascript
+
+/**
+ * A regular expression that captures the name and content of a shorthand HTML tag such as `<div id='foo'/>`.
+ */
+class ShorthandTagRecognizer extends RegExpLiteral {
+  ShorthandTagRecognizer() {
+    exists(RegExpSequence seq, RegExpGroup name, RegExpGroup content |
+      // `/.../g`
+      this.isGlobal() and
+      this = seq.getLiteral() and
+      // `/<.../`
+      seq.getChild(0).getConstantValue() = "<" and
+      // `/...\/>/`
+      seq.getLastChild().getPredecessor().getConstantValue() = "/" and
+      seq.getLastChild().getConstantValue() = ">" and
+      // `/...((...)...).../`
+      seq.getAChild() = content and
+      content.getNumber() = 1 and
+      name.getNumber() = 2 and
+      name = content.getChild(0).(RegExpSequence).getChild(0) and
+      // `/...(([a-z]+)...).../` or `/...(([a-z][...]*)...).../`
+      exists(RegExpQuantifier quant | name.getAChild*() = quant |
+        quant instanceof RegExpStar or
+        quant instanceof RegExpPlus
+      ) and
+      // `/...((...)[^>]*).../`
+      exists(RegExpCharacterClass lazy |
+        name.getSuccessor().(RegExpStar).getChild(0) = lazy and
+        lazy.isInverted() and
+        lazy.getAChild().getConstantValue() = ">"
+      )
+    )
+  }
+
+  /**
+   * Gets a data flow node that may refer to this regular expression.
+   */
+  DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
+    t.start() and
+    result = this.flow()
+    or
+    exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t))
+  }
+}
+
+from ShorthandTagRecognizer regexp, StringReplaceCall replace
+where
+  regexp.ref(DataFlow::TypeTracker::end()).flowsTo(replace.getArgument(0)) and
+  replace.getRawReplacement().mayHaveStringValue("<$1></$2>")
+select replace,
+  "This HTML tag expansion may disable earlier sanitizations as $@ may match unintended strings.",
+  regexp, "this regular expression"

javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/UnsafeHtmlExpansion.expected

@@ -0,0 +1,8 @@
+| UnsafeHtmlExpansion.js:6:2:9:2 | html.re ... nded\\n\\t) | This HTML tag expansion may disable earlier sanitizations as $@ may match unintended strings. | UnsafeHtmlExpansion.js:7:3:7:95 | /<(?!ar ... )\\/>/gi | this regular expression |
+| UnsafeHtmlExpansion.js:10:2:10:68 | html.re ... panded) | This HTML tag expansion may disable earlier sanitizations as $@ may match unintended strings. | UnsafeHtmlExpansion.js:10:15:10:57 | /<(([a- ... )\\/>/gi | this regular expression |
+| UnsafeHtmlExpansion.js:13:2:16:2 | html.re ... nded\\n\\t) | This HTML tag expansion may disable earlier sanitizations as $@ may match unintended strings. | UnsafeHtmlExpansion.js:14:3:14:75 | /<(?!ar ... )\\/>/gi | this regular expression |
+| UnsafeHtmlExpansion.js:17:2:17:48 | html.re ... panded) | This HTML tag expansion may disable earlier sanitizations as $@ may match unintended strings. | UnsafeHtmlExpansion.js:17:15:17:37 | /<(([\\w ... )\\/>/gi | this regular expression |
+| UnsafeHtmlExpansion.js:20:2:23:2 | html.re ... nded\\n\\t) | This HTML tag expansion may disable earlier sanitizations as $@ may match unintended strings. | UnsafeHtmlExpansion.js:21:3:21:76 | /<(?!ar ... )\\/>/gi | this regular expression |
+| UnsafeHtmlExpansion.js:24:2:24:49 | html.re ... panded) | This HTML tag expansion may disable earlier sanitizations as $@ may match unintended strings. | UnsafeHtmlExpansion.js:24:15:24:38 | /<(([\\w ... )\\/>/gi | this regular expression |
+| UnsafeHtmlExpansion.js:26:2:26:39 | html.re ... panded) | This HTML tag expansion may disable earlier sanitizations as $@ may match unintended strings. | UnsafeHtmlExpansion.js:2:23:2:45 | /<(([\\w ... )\\/>/gi | this regular expression |
+| UnsafeHtmlExpansion.js:30:2:30:37 | html.re ... panded) | This HTML tag expansion may disable earlier sanitizations as $@ may match unintended strings. | UnsafeHtmlExpansion.js:2:23:2:45 | /<(([\\w ... )\\/>/gi | this regular expression |

javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/UnsafeHtmlExpansion.js

@@ -0,0 +1,39 @@
+(function(){
+	let defaultPattern = /<(([\w:]+)[^>]*)\/>/gi;
+	let expanded = "<$1></$2>";
+
+	// lib1
+	html.replace(
+		/<(?!area|br|col|embed|hr|img|input|link|meta|param)(([a-z][^\/\0>\x20\t\r\n\f]*)[^>]*)\/>/gi,
+		expanded
+	); // NOT OK
+	html.replace(/<(([a-z][^\/\0>\x20\t\r\n\f]*)[^>]*)\/>/gi, expanded); // NOT OK
+
+	// lib2
+	html.replace(
+		/<(?!area|br|col|embed|hr|img|input|link|meta|param)(([\w:]+)[^>]*)\/>/gi,
+		expanded
+	); // NOT OK
+	html.replace(/<(([\w:]+)[^>]*)\/>/gi, expanded); // NOT OK
+
+	// lib3
+	html.replace(
+		/<(?!area|br|col|embed|hr|img|input|link|meta|param)(([\w:-]+)[^>]*)\/>/gi,
+		expanded
+	); // NOT OK
+	html.replace(/<(([\w:-]+)[^>]*)\/>/gi, expanded); // NOT OK
+
+	html.replace(defaultPattern, expanded); // NOT OK
+	function getPattern() {
+		return defaultPattern;
+	}
+	html.replace(getPattern(), expanded); // NOT OK
+
+	function getExpanded() {
+		return expanded;
+	}
+	html.replace(defaultPattern, getExpanded()); // NOT OK (but not tracking the expansion string)
+	html.replace(defaultPattern, something); // OK (possibly)
+	defaultPattern.match(something); // OK (possibly)
+	getPattern().match(something); // OK (possibly)
+});

javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/UnsafeHtmlExpansion.qlref

@@ -0,0 +1 @@
+Security/CWE-116/UnsafeHtmlExpansion.ql