diff --git a/change-notes/1.18/analysis-javascript.md b/change-notes/1.18/analysis-javascript.md index 0c31d5a2a06..8432ba66cb1 100644 --- a/change-notes/1.18/analysis-javascript.md +++ b/change-notes/1.18/analysis-javascript.md @@ -19,6 +19,9 @@ - [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) @@ -26,6 +29,8 @@ - [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) @@ -48,10 +53,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 index 07d94a4a616..b2bd08df0de 100644 --- 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 new file mode 100644 index 00000000000..9cf646efdf2 --- /dev/null +++ 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).*(?')) | 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 new file mode 100644 index 00000000000..824865b728c --- /dev/null +++ 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 new file mode 100644 index 00000000000..5de2325254e --- /dev/null +++ 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 new file mode 100644 index 00000000000..2154d4ef96d --- /dev/null +++ b/javascript/ql/test/library-tests/HtmlSanitizers/tst.js @@ -0,0 +1,38 @@ +function checkEscaped(str) { + if (str !== '<script>' && str !== '<script>' && str !== '<script>' && str !== '<script>') { + throw new Error('Not escaped: ' + str); + } +} +function checkStripped(str) { + if (str !== '') { + throw new Error('Not stripped: ' + str); + } +} +function checkNotEscaped(str) { + if (str !== '