github · semmle-qlci · Aug 13, 2018 · Jul 27, 2018 · Aug 10, 2018 · Aug 10, 2018
diff --git a/change-notes/1.18/analysis-javascript.md b/change-notes/1.18/analysis-javascript.md
@@ -19,13 +19,18 @@
   - [deepmerge](https://github.com/KyleAMathews/deepmerge)
   - [defaults-deep](https://github.com/jonschlinkert/defaults-deep)
   - [defaults](https://github.com/tmpvar/defaults)
+  - [ent](https://github.com/substack/node-ent)
+  - [entities](https://github.com/fb55/node-entities)
+  - [escape-goat](https://github.com/sindresorhus/escape-goat)
   - [express-jwt](https://github.com/auth0/express-jwt)
   - [express-session](https://github.com/expressjs/session)
   - [extend-shallow](https://github.com/jonschlinkert/extend-shallow)
   - [extend](https://github.com/justmoon/node-extend)
   - [extend2](https://github.com/eggjs/extend2)
   - [fast-json-parse](https://github.com/mcollina/fast-json-parse)
   - [forge](https://github.com/digitalbazaar/forge)
+  - [he](https://github.com/mathiasbynens/he)
+  - [html-entities](https://github.com/mdevils/node-html-entities)
   - [jquery](https://jquery.com)
   - [js-extend](https://github.com/vmattos/js-extend)
   - [json-parse-better-errors](https://github.com/zkat/json-parse-better-errors)
@@ -47,10 +52,14 @@
   - [q](http://documentup.com/kriskowal/q/)
   - [ramda](https://ramdajs.com)
   - [safe-json-parse](https://github.com/Raynos/safe-json-parse)
+  - [sanitize](https://github.com/pocketly/node-sanitize)
+  - [sanitizer](https://github.com/theSmaw/Caja-HTML-Sanitizer)
   - [smart-extend](https://github.com/danielkalen/smart-extend)
   - [underscore](https://underscorejs.org)
   - [util-extend](https://github.com/isaacs/util-extend)
   - [utils-merge](https://github.com/jaredhanson/utils-merge)
+  - [validator](https://github.com/chriso/validator.js)
+  - [xss](https://github.com/leizongmin/js-xss)
   - [xtend](https://github.com/Raynos/xtend)
 
 ## New queries

diff --git a/javascript/ql/src/javascript.qll b/javascript/ql/src/javascript.qll
@@ -21,6 +21,7 @@ import semmle.javascript.Externs
 import semmle.javascript.Files
 import semmle.javascript.Functions
 import semmle.javascript.HTML
+import semmle.javascript.HtmlSanitizers
 import semmle.javascript.JSDoc
 import semmle.javascript.JSON
 import semmle.javascript.JsonParsers

diff --git a/javascript/ql/src/semmle/javascript/HtmlSanitizers.qll b/javascript/ql/src/semmle/javascript/HtmlSanitizers.qll
@@ -0,0 +1,49 @@
+/**
+ * Provides classes for working with HTML sanitizers.
+ */
+import javascript
+
+/**
+ * A call that sanitizes HTML in a string, either by replacing
+ * meta characters with their HTML entities, or by removing
+ * certain HTML tags entirely.
+ */
+abstract class HtmlSanitizerCall extends DataFlow::CallNode {
+  /**
+   * Gets the data flow node referring to the input that gets sanitized.
+   */
+  abstract DataFlow::Node getInput();
+}
+
+/**
+ * Matches HTML sanitizers from known NPM packages as well as home-made sanitizers (matched by name).
+ */
+private class DefaultHtmlSanitizerCall extends HtmlSanitizerCall {
+  DefaultHtmlSanitizerCall() {
+    exists (DataFlow::SourceNode callee | this = callee.getACall() |
+      callee = DataFlow::moduleMember("ent", "encode") or
+      callee = DataFlow::moduleMember("entities", "encodeHTML") or
+      callee = DataFlow::moduleMember("entities", "encodeXML") or
+      callee = DataFlow::moduleMember("escape-goat", "escape") or
+      callee = DataFlow::moduleMember("he", "encode") or
+      callee = DataFlow::moduleMember("he", "escape") or
+      callee = DataFlow::moduleImport("sanitize-html") or
+      callee = DataFlow::moduleMember("sanitizer", "escape") or
+      callee = DataFlow::moduleMember("sanitizer", "sanitize") or
+      callee = DataFlow::moduleMember("validator", "escape") or
+      callee = DataFlow::moduleImport("xss") or
+      callee = DataFlow::moduleMember("xss-filters", _) or
+      callee = LodashUnderscore::member("escape") or
+      exists (string name | name = "encode" or name = "encodeNonUTF" |
+        callee = DataFlow::moduleMember("html-entities", _).getAnInstantiation().getAPropertyRead(name) or
+        callee = DataFlow::moduleMember("html-entities", _).getAPropertyRead(name))
+    )
+    or
+    // Match home-made sanitizers by name.
+    exists (string calleeName | calleeName = getCalleeName() |
+      calleeName.regexpMatch("(?i).*html.*") and
+      calleeName.regexpMatch("(?i).*(?<!un)(saniti[sz]|escape|strip).*"))
+  }
+
+  override DataFlow::Node getInput() { result = getArgument(0) }
+}
diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/CodeInjection.qll b/javascript/ql/src/semmle/javascript/security/dataflow/CodeInjection.qll
@@ -43,12 +43,7 @@ module CodeInjection {
 
     override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node trg) {
       // HTML sanitizers are insufficient protection against code injection
-      exists(CallExpr htmlSanitizer, string calleeName |
-        calleeName = htmlSanitizer.getCalleeName() and
-        calleeName.regexpMatch("(?i).*html.*") and
-        calleeName.regexpMatch("(?i).*(saniti[sz]|escape|strip).*") and
-        trg.asExpr() = htmlSanitizer and src.asExpr() = htmlSanitizer.getArgument(0)
-      )
+      src = trg.(HtmlSanitizerCall).getInput()
     }
   }
 

diff --git a/javascript/ql/test/library-tests/HtmlSanitizers/HtmlSanitizerCalls.expected b/javascript/ql/test/library-tests/HtmlSanitizers/HtmlSanitizerCalls.expected
@@ -0,0 +1,19 @@
+| tst.js:17:1:17:47 | checkEs ... ipt>')) | OK |
+| tst.js:18:1:18:56 | checkEs ... ipt>')) | OK |
+| tst.js:19:1:19:55 | checkEs ... ipt>')) | OK |
+| tst.js:20:1:20:55 | checkEs ... ipt>')) | OK |
+| tst.js:21:1:21:46 | checkEs ... ipt>')) | OK |
+| tst.js:22:1:22:46 | checkEs ... ipt>')) | OK |
+| tst.js:23:1:23:50 | checkEs ... ipt>')) | OK |
+| tst.js:24:1:24:53 | checkEs ... ipt>')) | OK |
+| tst.js:25:1:25:54 | checkEs ... ipt>')) | OK |
+| tst.js:26:1:26:53 | checkEs ... ipt>')) | OK |
+| tst.js:27:1:27:40 | checkEs ... ipt>')) | OK |
+| tst.js:28:1:28:59 | checkEs ... ipt>')) | OK |
+| tst.js:29:1:29:51 | checkSt ... ipt>')) | OK |
+| tst.js:30:1:30:56 | checkSt ... ipt>')) | OK |
+| tst.js:33:1:33:47 | checkEs ... ipt>')) | OK |
+| tst.js:34:1:34:53 | checkEs ... ipt>')) | OK |
+| tst.js:35:1:35:41 | checkEs ... ipt>')) | OK |
+| tst.js:36:1:36:47 | checkEs ... ipt>')) | OK |
+| tst.js:38:1:38:58 | checkNo ... ipt>')) | OK |
diff --git a/javascript/ql/test/library-tests/HtmlSanitizers/HtmlSanitizerCalls.ql b/javascript/ql/test/library-tests/HtmlSanitizers/HtmlSanitizerCalls.ql
@@ -0,0 +1,25 @@
+import javascript
+
+class Assertion extends DataFlow::CallNode {
+  Assertion() {
+    getCalleeName() = "checkEscaped" or
+    getCalleeName() = "checkStripped" or
+    getCalleeName() = "checkNotEscaped"
+  }
+
+  predicate shouldBeSanitizer() {
+    getCalleeName() != "checkNotEscaped"
+  }
+
+  string getMessage() {
+    if shouldBeSanitizer() and not getArgument(0) instanceof HtmlSanitizerCall then
+      result = "Should be marked as sanitizer"
+    else if not shouldBeSanitizer() and getArgument(0) instanceof HtmlSanitizerCall then
+      result = "Should not be marked as sanitizer"
+    else
+      result = "OK"
+  }
+}
+
+from Assertion assertion
+select assertion, assertion.getMessage()
diff --git a/javascript/ql/test/library-tests/HtmlSanitizers/package.json b/javascript/ql/test/library-tests/HtmlSanitizers/package.json
@@ -0,0 +1,17 @@
+{
+  "private": true,
+  "dependencies": {
+    "ent": "^2.2.0",
+    "entities": "^1.1.1",
+    "escape-goat": "^1.3.0",
+    "he": "^1.1.1",
+    "html-entities": "^1.2.1",
+    "lodash": "^4.17.10",
+    "sanitize-html": "^1.18.2",
+    "sanitizer": "^0.1.3",
+    "underscore": "^1.9.1",
+    "validator": "^10.4.0",
+    "xss": "^1.0.3",
+    "xss-filters": "^1.2.7"
+  }
+}
diff --git a/javascript/ql/test/library-tests/HtmlSanitizers/tst.js b/javascript/ql/test/library-tests/HtmlSanitizers/tst.js
@@ -0,0 +1,38 @@
+function checkEscaped(str) {
+  if (str !== '&lt;script&gt;' && str !== '&#x3C;script&#x3E;' && str !== '&#60;script&#62;' && str !== '&lt;script>') {
+    throw new Error('Not escaped: ' + str);
+  }
+}
+function checkStripped(str) {
+  if (str !== '') {
+    throw new Error('Not stripped: ' + str);
+  }
+}
+function checkNotEscaped(str) {
+  if (str !== '<script>') {
+    throw new Error('Escaped: ' + str);
+  }
+}
+
+checkEscaped(require('ent').encode('<script>'));
+checkEscaped(require('entities').encodeHTML('<script>'));
+checkEscaped(require('entities').encodeXML('<script>'));
+checkEscaped(require('escape-goat').escape('<script>'));
+checkEscaped(require('he').encode('<script>'));
+checkEscaped(require('he').escape('<script>'));
+checkEscaped(require('lodash').escape('<script>'));
+checkEscaped(require('sanitizer').escape('<script>'));
+checkEscaped(require('underscore').escape('<script>'));
+checkEscaped(require('validator').escape('<script>'));
+checkEscaped(require('xss')('<script>'));
+checkEscaped(require('xss-filters').inHTMLData('<script>'));
+checkStripped(require('sanitize-html')('<script>'));
+checkStripped(require('sanitizer').sanitize('<script>'));
+
+let Entities = require('html-entities').Html5Entities;
+checkEscaped(new Entities().encode('<script>'));
+checkEscaped(new Entities().encodeNonUTF('<script>'));
+checkEscaped(Entities.encode('<script>'));
+checkEscaped(Entities.encodeNonUTF('<script>'));
+
+checkNotEscaped(new Entities().encodeNonASCII('<script>'));