mozilla
/
eslint-plugin-no-unsanitized
зеркало из
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Custom rules for configuring rules. Fixes #29 (#51)

This commit is contained in:
Jonathan Kingston 2017-04-21 15:52:12 +01:00 коммит произвёл Frederik
Родитель 570ea4debe
Коммит db42f8173c
9 изменённых файлов: 440 добавлений и 53 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

1
.gitignore поставляемый
Просмотреть файл

@ -1,2 +1,3 @@
node_modules
.idea
*.swp

39
README.md
Просмотреть файл

@ -1,5 +1,5 @@
[![Build Status](https://travis-ci.org/mozilla/eslint-plugin-no-unsanitized.svg?branch=master)](https://travis-ci.org/mozilla/eslint-plugin-no-unsanitized)
# Disallow unsanitized DOM access (no-unsanitized)
# Disallow unsanitized code (no-unsanitized)
These rules disallow unsafe coding practices that may result into security vulnerabilities. We will disallow assignments to innerHTML as well as calls to insertAdjacentHTML without the use of a pre-defined escaping function. The escaping functions must be called with a template string. The function names are hardcoded as `Sanitizer.escapeHTML` and `escapeHTML`.
@ -31,6 +31,7 @@ This rule is being used within Mozilla to maintain and improve the security of o
In your eslint.json file enable this rule with the following:
```
{
"plugins": ["no-unsanitized"],
@ -40,3 +41,39 @@ In your eslint.json file enable this rule with the following:
}
}
```
## Advanced configuration
```js
{
"plugins": ["no-unsanitized"],
"env": {
"no-unsanitized/method": [
"error",
{
disableDefault: true,
escape: {
taggedTemplates: ["safeHTML"]
}
},
{
html: {
properties: [0]
}
}
],
"no-unsanitized/method": [
"error",
{
},
{
innerHTML: {
objectMatches: ["document.*"]
}
}
]
}
}
```
[To see all available options vitit](./SCHEMA.md)

43
SCHEMA.md Normal file
Просмотреть файл

@ -0,0 +1,43 @@
## Schema
```js
{
"plugins": ["no-unsanitized"],
"env": {
// $ruleName is either "method" or "property"
"no-unsanitized/$ruleName": ["errorlevel",
// This object is optional would use default config for all default ruleChecks
{
// optional, omitting or length 0 would be considered permitting all objectNames
objectMatches: ["$stringRegex"],
// optional, removes default $ruleCheckName's provided by this rule
disableDefault: $boolean,
// optional, if omitted would disallow use of matched "$ruleCheckName" prop/method
escape: {
// optional: Tagged template permitted as arguments/assignments
taggedTemplates: ["$taggedTemplateFunctionName"],
// optional: method permitted as arguments/assignments
methods: ["$MethodName"]
}
},
// optional would use default ruleChecks, will merge each $ruleCheckName with default rule setup so user doesn't have to specify properties if already defined
{
"$ruleCheckName": {
// optional, same as above and overrides parent even if blank array
objectMatches: ["$stringRegex"],
// optional, same as above, overrides parent even if blank object
escape: {...}
// indices to check for arguments passed to a method call. Only applies to $ruleName="method"
properties: [...]
}
...
}
]
}
}
```

195
lib/ruleHelper.js
Просмотреть файл

@ -24,9 +24,14 @@ RuleHelper.prototype = {
* Using the Esprima Demo, for example: http://esprima.org/demo/parse.html
*
* @param {Object} expression Checks whether this node is an allowed expression.
* @param {Object} escapeObject contains keys "methods" and "taggedTemplates" which are arrays of strings
* of matching escaping function names.
* @returns {boolean}
*/
allowedExpression(expression) {
allowedExpression(expression, escapeObject) {
if (!escapeObject) {
escapeObject = {};
}
/*
expression = { right-hand side of innerHTML or 2nd param to insertAdjacentHTML
*/
@ -49,30 +54,18 @@ RuleHelper.prototype = {
// check for ${..} expressions
for (let e = 0; e < expression.expressions.length; e++) {
const templateExpression = expression.expressions[e];
if (!this.allowedExpression(templateExpression)) {
if (!this.allowedExpression(templateExpression, escapeObject)) {
allowed = false;
break;
}
}
} else if (expression.type === "TaggedTemplateExpression") {
//TODO avoid code-duplication with CallExpression-case below.
const functionName = this.context.getSource(expression.tag);
if (VALID_ESCAPERS.indexOf(functionName) !== -1) {
allowed = true;
} else {
allowed = false;
}
} else if ((expression.type === "CallExpression") &&
(expression.callee.property || expression.callee.name)) {
const funcName = expression.callee.name || expression.callee.property.name;
if (funcName && VALID_UNWRAPPERS.indexOf(funcName) !== -1) {
allowed = true;
} else {
allowed = false;
}
allowed = this.isAllowedCallExpression(expression.tag, escapeObject.taggedTemplates || VALID_ESCAPERS);
} else if (expression.type === "CallExpression") {
allowed = this.isAllowedCallExpression(expression.callee, escapeObject.methods || VALID_UNWRAPPERS);
} else if (expression.type === "BinaryExpression") {
allowed = ((this.allowedExpression(expression.left))
&& (this.allowedExpression(expression.right)));
allowed = ((this.allowedExpression(expression.left, escapeObject))
&& (this.allowedExpression(expression.right, escapeObject)));
} else {
// everything that doesn't match is unsafe:
allowed = false;
@ -80,6 +73,170 @@ RuleHelper.prototype = {
return allowed;
},
/**
* Check if a callee is in the list allowed sanitizers
* @param {Object} callee function that is being called expression.tag or expression.callee
* @param {Array} allowedSanitizers List of valid wrapping expressions
* @returns {boolean}
*/
isAllowedCallExpression(callee, allowedSanitizers) {
const funcName = this.getCodeName(callee);
let allowed = false;
if (funcName && allowedSanitizers.indexOf(funcName) !== -1) {
allowed = true;
}
return allowed;
},
/**
* Captures safely any new node types that have been missed and throw when we don't support them
* this normalizes the passed in identifier type to return the same shape
*/
normalizeMethodCall(node) {
let methodName;
let objectName;
switch (node.type) {
case "Identifier":
methodName = node.name;
break;
case "MemberExpression":
methodName = node.property.name;
objectName = node.object.name;
break;
case "ArrowFunctionExpression":
methodName = "";
break;
default:
this.reportUnsupported(node, "Unexpected callable", `unexpected ${node.type} in normalizeMethodName`);
}
return {
objectName,
methodName
};
},
/**
* Returns functionName or objectName.methodName of an expression
* @param {Object} node
* @returns {String} nice name to expression call
*/
getCodeName(node) {
const normalizedMethodCall = this.normalizeMethodCall(node);
let codeName = normalizedMethodCall.methodName;
if (normalizedMethodCall.objectName) {
codeName = `${normalizedMethodCall.objectName}.${codeName}`;
}
return codeName;
},
/**
* Checks to see if a method or function should be called
* If objectMatches isn't present or blank array the code should not be checked
* If we do have object filters and the call is a function then it should not be checked
* Checks if there are objectMatches we need to apply
* @param {Object} node call expression node
* @param {Object} objectMatches strings that are checked as regex to match an object name
* @returns {Boolean} if to run checks expression
*/
shouldCheckMethodCall(node, objectMatches) {
const normalizedMethodCall = this.normalizeMethodCall(node.callee);
let matched = false;
// If objectMatches isn't present we should match all
if (!objectMatches) {
return true;
}
// if blank array the code should not be checked, this is a quick way to disable rules
// TODO should we make this match all instead and let the $ruleCheck be false instead?
if (objectMatches.length === 0) {
return false;
}
// If we do have object filters and the call is a function then it should not be checked
if ("objectName" in normalizedMethodCall) {
for (const objectMatch of objectMatches) {
const match = new RegExp(objectMatch, "gi");
if (normalizedMethodCall.objectName.match(match)) {
matched = true;
break;
}
}
}
// if we don't have a objectName return false as bare function call
// if we didn't match also return false
return matched;
},
/**
* Algorithm used to decide on merging ruleChecks with this.context
* @returns {Object} merged ruleChecks
*/
combineRuleChecks(defaultRuleChecks) {
const parentRuleChecks = this.context.options[0] || {};
let childRuleChecks = Object.assign({}, this.context.options[1]);
const ruleCheckOutput = {};
if (!("defaultDisable" in parentRuleChecks)
|| !parentRuleChecks.defaultDisable) {
childRuleChecks = Object.assign({}, defaultRuleChecks, childRuleChecks);
}
// If we have defined child rules lets ignore default rules
Object.keys(childRuleChecks).forEach((ruleCheckKey) => {
// However if they have missing keys merge with default
const ruleCheck = Object.assign({},
defaultRuleChecks[ruleCheckKey],
parentRuleChecks,
childRuleChecks[ruleCheckKey]);
ruleCheckOutput[ruleCheckKey] = ruleCheck;
});
return ruleCheckOutput;
},
/**
* Runs the checks against a CallExpression
* @param {Object} node call expression node
* @param {Object} defaultRuleChecks default rules to merge with this.context
*/
checkMethod(node, defaultRuleChecks) {
const ruleChecks = this.combineRuleChecks(defaultRuleChecks);
const normalizeMethodCall = this.normalizeMethodCall(node.callee);
const methodName = normalizeMethodCall.methodName;
if (ruleChecks.hasOwnProperty(methodName)) {
const ruleCheck = ruleChecks[methodName];
if (!Array.isArray(ruleCheck.properties)) {
this.context.report(node, `Method check requires properties array in eslint rule ${methodName}`);
return;
}
ruleCheck.properties.forEach((propertyId) => {
const argument = node.arguments[propertyId];
if (this.shouldCheckMethodCall(node, ruleCheck.objectMatches)
&& !this.allowedExpression(argument, ruleCheck.escape)) {
this.context.report(node, `Unsafe call to ${this.getCodeName(node.callee)} for argument ${propertyId}`);
}
});
}
},
/**
* Runs the checks against an assignment expression
* @param {Object} node assignment expression node
* @param {Object} defaultRuleChecks default rules to merge with this.context
*/
checkProperty(node, defaultRuleChecks) {
const ruleChecks = this.combineRuleChecks(defaultRuleChecks);
if (ruleChecks.hasOwnProperty(node.left.property.name)) {
const ruleCheck = ruleChecks[node.left.property.name];
if (!this.allowedExpression(node.right, ruleCheck.escape)) {
this.context.report(node, `Unsafe assignment to ${node.left.property.name}`);
}
}
},
reportUnsupported(node, reason, errorTitle) {
const bugPath = `https://github.com/mozilla/eslint-plugin-no-unsanitized/issues/new?title=${encodeURIComponent(errorTitle)}`;
this.context.report(node, `Error in no-unsanitized: ${reason}. Please report a minimal code snippet to the developers at ${bugPath}`);

43
lib/rules/method.js
Просмотреть файл

@ -12,6 +12,31 @@ const RuleHelper = require("../ruleHelper");
// Rule Definition
//------------------------------------------------------------------------------
const defaultRuleChecks = {
// check second parameter to .insertAdjacentHTML()
insertAdjacentHTML: {
properties: [1]
},
// check first parameter to .write(), as long as the preceeding object matches the regex "document"
write: {
objectMatches: [
"document"
],
properties: [0]
},
// check first parameter to .writeLn(), as long as the preceeding object matches the regex "document"
writeln: {
objectMatches: [
"document"
],
properties: [0]
}
};
module.exports = {
meta: {
docs: {
@ -34,20 +59,8 @@ module.exports = {
switch(node.callee.type) {
case "Identifier":
case "MemberExpression":
if ("property" in node.callee && node.arguments.length > 0) {
if (node.callee.property.name === "insertAdjacentHTML") {
if (!ruleHelper.allowedExpression(node.arguments[1])) {
context.report(node, "Unsafe call to insertAdjacentHTML");
}
} else if (context.getSource(node.callee) === "document.write") {
if (!ruleHelper.allowedExpression(node.arguments[0])) {
context.report(node, "Unsafe call to document.write");
}
} else if (context.getSource(node.callee) === "document.writeln") {
if (!ruleHelper.allowedExpression(node.arguments[0])) {
context.report(node, "Unsafe call to document.writeln");
}
}
if (node.arguments.length > 0) {
ruleHelper.checkMethod(node, defaultRuleChecks);
}
break;
@ -58,6 +71,8 @@ module.exports = {
break;
case "Super":
break;
// If we don't cater for this expression throw an error
default:
context.reportUnsupported(node, "Unexpected Callee", "Unsupported Callee for CallExpression");
}

21
lib/rules/property.js
Просмотреть файл

@ -12,6 +12,18 @@ const RuleHelper = require("../ruleHelper");
// Rule Definition
//------------------------------------------------------------------------------
const defaultRuleChecks = {
// Check unsafe assignment to innerHTML
innerHTML: {
},
// Check unsafe assignment to outerHTML
outerHTML: {
}
};
module.exports = {
meta: {
docs: {
@ -47,15 +59,10 @@ module.exports = {
if (CHECK_REQUIRED_OPERATORS.indexOf(node.operator) === -1) {
ruleHelper.reportUnsupported(node, "Unexpected Assignment", "Unsupported Operator for AssignmentExpression");
}
if ((node.left.property.name === "innerHTML") ||
(node.left.property.name === "outerHTML")) {
if (!ruleHelper.allowedExpression(node.right)) {
context.report(node, `Unsafe assignment to ${node.left.property.name}`);
}
}
ruleHelper.checkProperty(node, defaultRuleChecks);
}
}
},
}
};
}
};

2
package.json
Просмотреть файл

@ -30,6 +30,6 @@
},
"scripts":{
"test": "./node_modules/.bin/mocha tests/rules/",
"lint": "./node_modules/.bin/eslint index.js lib/**/*.js tests/**/*.js"
"lint": "./node_modules/.bin/eslint index.js lib/*.js lib/**/*.js tests/**/*.js"
}
}

122
tests/rules/method.js
Просмотреть файл

@ -24,12 +24,6 @@ eslintTester.run("method", rule, {
// XXX this does not find z['innerHTML'] and the like.
valid: [
// testing unwrapSafeHTML spread
{
code: "this.imeList.innerHTML = Sanitizer.unwrapSafeHTML(...listHtml);",
parserOptions: { ecmaVersion: 6 }
},
// tests for insertAdjacentHTML calls
{
code: "n.insertAdjacentHTML('afterend', 'meh');",
@ -57,6 +51,51 @@ eslintTester.run("method", rule, {
code: "document.writeln(Sanitizer.escapeHTML`<em>${evil}</em>`);",
parserOptions: { ecmaVersion: 6 }
},
{
code: "otherNodeWeDontCheckFor.writeln(evil);",
parserOptions: { ecmaVersion: 6 }
},
// Native method (Check customize code doesn't include these)
{
code: "document.toString(evil);"
},
{
code: "document.write(escaper(x))",
options: [
{
escape: {
methods: ["escaper"]
}
}
]
},
// Checking write can be overriden
{
code: "document.write(evilest)",
options: [
{
objectMatches: ["document", "documentFun"]
},
{
write: {
objectMatches: ["thing"]
}
}
]
},
// Checking disableDefault can remove the default rules
{
code: "document.write(evil)",
options: [
{
defaultDisable: true
}
]
}
],
// Examples of code that should trigger the rule
@ -72,7 +111,7 @@ eslintTester.run("method", rule, {
code: "node.insertAdjacentHTML('beforebegin', htmlString);",
errors: [
{
message: "Unsafe call to insertAdjacentHTML",
message: "Unsafe call to node.insertAdjacentHTML for argument 1",
type: "CallExpression"
}
]
@ -81,7 +120,7 @@ eslintTester.run("method", rule, {
code: "node.insertAdjacentHTML('beforebegin', template.getHTML());",
errors: [
{
message: "Unsafe call to insertAdjacentHTML",
message: "Unsafe call to node.insertAdjacentHTML for argument 1",
type: "CallExpression"
}
]
@ -92,7 +131,25 @@ eslintTester.run("method", rule, {
code: "document.write('<span>'+ htmlInput + '</span>');",
errors: [
{
message: "Unsafe call to document.write",
message: "Unsafe call to document.write for argument 0",
type: "CallExpression"
}
]
},
{
code: "documentish.write('<span>'+ htmlInput + '</span>');",
errors: [
{
message: "Unsafe call to documentish.write for argument 0",
type: "CallExpression"
}
]
},
{
code: "documentIframe.write('<span>'+ htmlInput + '</span>');",
errors: [
{
message: "Unsafe call to documentIframe.write for argument 0",
type: "CallExpression"
}
]
@ -101,11 +158,56 @@ eslintTester.run("method", rule, {
code: "document.writeln(evil);",
errors: [
{
message: "Unsafe call to document.writeln",
message: "Unsafe call to document.writeln for argument 0",
type: "CallExpression"
}
]
},
// Broken config
{
code: "b.boop(pie)",
options: [
{
},
{
boop: {
}
}
],
errors: [
{
message: "Method check requires properties array in eslint rule boop",
type: "CallExpression"
}
]
},
// Checking disableDefault can remove the default rules but also add more
{
code: "document.write(evil); b.thing(x); b.other(me);",
options: [
{
defaultDisable: true
},
{
thing: {
},
other: {
properties: [0]
}
}
],
errors: [
{
message: "Method check requires properties array in eslint rule thing",
type: "CallExpression"
},
{
message: "Unsafe call to b.other for argument 0",
type: "CallExpression"
}
]
}
]
});

27
tests/rules/property.js
Просмотреть файл

@ -111,7 +111,19 @@ eslintTester.run("property", rule, {
{
code: "w.innerHTML = `<span>${'lulz'+'meh'}</span>`;",
parserOptions: { ecmaVersion: 6 }
}
},
// testing unwrapSafeHTML spread
{
code: "this.imeList.innerHTML = Sanitizer.unwrapSafeHTML(...listHtml);",
parserOptions: { ecmaVersion: 6 }
},
// Native method (Check customize code doesn't include these)
{
code: "document.toString = evil;"
},
],
// Examples of code that should trigger the rule
@ -261,6 +273,19 @@ eslintTester.run("property", rule, {
parserOptions: { ecmaVersion: 6 }
},
// testing unwrapSafeHTML spread sanitizer typo
{
code: "this.imeList.innerHTML = Sanitizer.unrapSafeHTML(...listHtml);",
errors: [
{
message: "Unsafe assignment to innerHTML",
type: "AssignmentExpression"
}
],
parserOptions: { ecmaVersion: 6 }
},
// the previous override for manual review and legacy code is now invalid
{
code: "g.innerHTML = potentiallyUnsafe; // a=legacy, bug 1155131",