Skip to content

Commit c0fe0a1

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request #46 from asger-semmle/html-sanitizers
Approved by xiemaisi
2 parents 3d0748c + 8074786 commit c0fe0a1

File tree

8 files changed

+159
-6
lines changed

8 files changed

+159
-6
lines changed

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,18 @@
1919
- [deepmerge](https://github.com/KyleAMathews/deepmerge)
2020
- [defaults-deep](https://github.com/jonschlinkert/defaults-deep)
2121
- [defaults](https://github.com/tmpvar/defaults)
22+
- [ent](https://github.com/substack/node-ent)
23+
- [entities](https://github.com/fb55/node-entities)
24+
- [escape-goat](https://github.com/sindresorhus/escape-goat)
2225
- [express-jwt](https://github.com/auth0/express-jwt)
2326
- [express-session](https://github.com/expressjs/session)
2427
- [extend-shallow](https://github.com/jonschlinkert/extend-shallow)
2528
- [extend](https://github.com/justmoon/node-extend)
2629
- [extend2](https://github.com/eggjs/extend2)
2730
- [fast-json-parse](https://github.com/mcollina/fast-json-parse)
2831
- [forge](https://github.com/digitalbazaar/forge)
32+
- [he](https://github.com/mathiasbynens/he)
33+
- [html-entities](https://github.com/mdevils/node-html-entities)
2934
- [jquery](https://jquery.com)
3035
- [js-extend](https://github.com/vmattos/js-extend)
3136
- [json-parse-better-errors](https://github.com/zkat/json-parse-better-errors)
@@ -48,10 +53,14 @@
4853
- [q](http://documentup.com/kriskowal/q/)
4954
- [ramda](https://ramdajs.com)
5055
- [safe-json-parse](https://github.com/Raynos/safe-json-parse)
56+
- [sanitize](https://github.com/pocketly/node-sanitize)
57+
- [sanitizer](https://github.com/theSmaw/Caja-HTML-Sanitizer)
5158
- [smart-extend](https://github.com/danielkalen/smart-extend)
5259
- [underscore](https://underscorejs.org)
5360
- [util-extend](https://github.com/isaacs/util-extend)
5461
- [utils-merge](https://github.com/jaredhanson/utils-merge)
62+
- [validator](https://github.com/chriso/validator.js)
63+
- [xss](https://github.com/leizongmin/js-xss)
5564
- [xtend](https://github.com/Raynos/xtend)
5665

5766
## New queries

‎javascript/ql/src/javascript.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import semmle.javascript.Externs
2121
import semmle.javascript.Files
2222
import semmle.javascript.Functions
2323
import semmle.javascript.HTML
24+
import semmle.javascript.HtmlSanitizers
2425
import semmle.javascript.JSDoc
2526
import semmle.javascript.JSON
2627
import semmle.javascript.JsonParsers

‎javascript/ql/src/semmle/javascript/HtmlSanitizers.qll‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* Provides classes for working with HTML sanitizers.
3+
*/
4+
import javascript
5+
6+
/**
7+
* A call that sanitizes HTML in a string, either by replacing
8+
* meta characters with their HTML entities, or by removing
9+
* certain HTML tags entirely.
10+
*/
11+
abstract class HtmlSanitizerCall extends DataFlow::CallNode {
12+
/**
13+
* Gets the data flow node referring to the input that gets sanitized.
14+
*/
15+
abstract DataFlow::Node getInput();
16+
}
17+
18+
/**
19+
* Matches HTML sanitizers from known NPM packages as well as home-made sanitizers (matched by name).
20+
*/
21+
private class DefaultHtmlSanitizerCall extends HtmlSanitizerCall {
22+
DefaultHtmlSanitizerCall() {
23+
exists (DataFlow::SourceNode callee | this = callee.getACall() |
24+
callee = DataFlow::moduleMember("ent", "encode") or
25+
callee = DataFlow::moduleMember("entities", "encodeHTML") or
26+
callee = DataFlow::moduleMember("entities", "encodeXML") or
27+
callee = DataFlow::moduleMember("escape-goat", "escape") or
28+
callee = DataFlow::moduleMember("he", "encode") or
29+
callee = DataFlow::moduleMember("he", "escape") or
30+
callee = DataFlow::moduleImport("sanitize-html") or
31+
callee = DataFlow::moduleMember("sanitizer", "escape") or
32+
callee = DataFlow::moduleMember("sanitizer", "sanitize") or
33+
callee = DataFlow::moduleMember("validator", "escape") or
34+
callee = DataFlow::moduleImport("xss") or
35+
callee = DataFlow::moduleMember("xss-filters", _) or
36+
callee = LodashUnderscore::member("escape") or
37+
exists (string name | name = "encode" or name = "encodeNonUTF" |
38+
callee = DataFlow::moduleMember("html-entities", _).getAnInstantiation().getAPropertyRead(name) or
39+
callee = DataFlow::moduleMember("html-entities", _).getAPropertyRead(name))
40+
)
41+
or
42+
// Match home-made sanitizers by name.
43+
exists (string calleeName | calleeName = getCalleeName() |
44+
calleeName.regexpMatch("(?i).*html.*") and
45+
calleeName.regexpMatch("(?i).*(?<!un)(saniti[sz]|escape|strip).*"))
46+
}
47+
48+
override DataFlow::Node getInput() { result = getArgument(0) }
49+
}

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,7 @@ module CodeInjection {
4343

4444
override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node trg) {
4545
// HTML sanitizers are insufficient protection against code injection
46-
exists(CallExpr htmlSanitizer, string calleeName |
47-
calleeName = htmlSanitizer.getCalleeName() and
48-
calleeName.regexpMatch("(?i).*html.*") and
49-
calleeName.regexpMatch("(?i).*(saniti[sz]|escape|strip).*") and
50-
trg.asExpr() = htmlSanitizer and src.asExpr() = htmlSanitizer.getArgument(0)
51-
)
46+
src = trg.(HtmlSanitizerCall).getInput()
5247
}
5348
}
5449

‎javascript/ql/test/library-tests/HtmlSanitizers/HtmlSanitizerCalls.expected‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
| tst.js:17:1:17:47 | checkEs ... ipt>')) | OK |
2+
| tst.js:18:1:18:56 | checkEs ... ipt>')) | OK |
3+
| tst.js:19:1:19:55 | checkEs ... ipt>')) | OK |
4+
| tst.js:20:1:20:55 | checkEs ... ipt>')) | OK |
5+
| tst.js:21:1:21:46 | checkEs ... ipt>')) | OK |
6+
| tst.js:22:1:22:46 | checkEs ... ipt>')) | OK |
7+
| tst.js:23:1:23:50 | checkEs ... ipt>')) | OK |
8+
| tst.js:24:1:24:53 | checkEs ... ipt>')) | OK |
9+
| tst.js:25:1:25:54 | checkEs ... ipt>')) | OK |
10+
| tst.js:26:1:26:53 | checkEs ... ipt>')) | OK |
11+
| tst.js:27:1:27:40 | checkEs ... ipt>')) | OK |
12+
| tst.js:28:1:28:59 | checkEs ... ipt>')) | OK |
13+
| tst.js:29:1:29:51 | checkSt ... ipt>')) | OK |
14+
| tst.js:30:1:30:56 | checkSt ... ipt>')) | OK |
15+
| tst.js:33:1:33:47 | checkEs ... ipt>')) | OK |
16+
| tst.js:34:1:34:53 | checkEs ... ipt>')) | OK |
17+
| tst.js:35:1:35:41 | checkEs ... ipt>')) | OK |
18+
| tst.js:36:1:36:47 | checkEs ... ipt>')) | OK |
19+
| tst.js:38:1:38:58 | checkNo ... ipt>')) | OK |

‎javascript/ql/test/library-tests/HtmlSanitizers/HtmlSanitizerCalls.ql‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import javascript
2+
3+
class Assertion extends DataFlow::CallNode {
4+
Assertion() {
5+
getCalleeName() = "checkEscaped" or
6+
getCalleeName() = "checkStripped" or
7+
getCalleeName() = "checkNotEscaped"
8+
}
9+
10+
predicate shouldBeSanitizer() {
11+
getCalleeName() != "checkNotEscaped"
12+
}
13+
14+
string getMessage() {
15+
if shouldBeSanitizer() and not getArgument(0) instanceof HtmlSanitizerCall then
16+
result = "Should be marked as sanitizer"
17+
else if not shouldBeSanitizer() and getArgument(0) instanceof HtmlSanitizerCall then
18+
result = "Should not be marked as sanitizer"
19+
else
20+
result = "OK"
21+
}
22+
}
23+
24+
from Assertion assertion
25+
select assertion, assertion.getMessage()

‎javascript/ql/test/library-tests/HtmlSanitizers/package.json‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"private": true,
3+
"dependencies": {
4+
"ent": "^2.2.0",
5+
"entities": "^1.1.1",
6+
"escape-goat": "^1.3.0",
7+
"he": "^1.1.1",
8+
"html-entities": "^1.2.1",
9+
"lodash": "^4.17.10",
10+
"sanitize-html": "^1.18.2",
11+
"sanitizer": "^0.1.3",
12+
"underscore": "^1.9.1",
13+
"validator": "^10.4.0",
14+
"xss": "^1.0.3",
15+
"xss-filters": "^1.2.7"
16+
}
17+
}

‎javascript/ql/test/library-tests/HtmlSanitizers/tst.js‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
function checkEscaped(str) {
2+
if (str !== '&lt;script&gt;' && str !== '&#x3C;script&#x3E;' && str !== '&#60;script&#62;' && str !== '&lt;script>') {
3+
throw new Error('Not escaped: ' + str);
4+
}
5+
}
6+
function checkStripped(str) {
7+
if (str !== '') {
8+
throw new Error('Not stripped: ' + str);
9+
}
10+
}
11+
function checkNotEscaped(str) {
12+
if (str !== '<script>') {
13+
throw new Error('Escaped: ' + str);
14+
}
15+
}
16+
17+
checkEscaped(require('ent').encode('<script>'));
18+
checkEscaped(require('entities').encodeHTML('<script>'));
19+
checkEscaped(require('entities').encodeXML('<script>'));
20+
checkEscaped(require('escape-goat').escape('<script>'));
21+
checkEscaped(require('he').encode('<script>'));
22+
checkEscaped(require('he').escape('<script>'));
23+
checkEscaped(require('lodash').escape('<script>'));
24+
checkEscaped(require('sanitizer').escape('<script>'));
25+
checkEscaped(require('underscore').escape('<script>'));
26+
checkEscaped(require('validator').escape('<script>'));
27+
checkEscaped(require('xss')('<script>'));
28+
checkEscaped(require('xss-filters').inHTMLData('<script>'));
29+
checkStripped(require('sanitize-html')('<script>'));
30+
checkStripped(require('sanitizer').sanitize('<script>'));
31+
32+
let Entities = require('html-entities').Html5Entities;
33+
checkEscaped(new Entities().encode('<script>'));
34+
checkEscaped(new Entities().encodeNonUTF('<script>'));
35+
checkEscaped(Entities.encode('<script>'));
36+
checkEscaped(Entities.encodeNonUTF('<script>'));
37+
38+
checkNotEscaped(new Entities().encodeNonASCII('<script>'));

0 commit comments

Comments
 (0)