diff --git a/README.md b/README.md index d48df46..ebc6222 100644 --- a/README.md +++ b/README.md @@ -44,6 +44,7 @@ You can also used the below Shareable Config files using flat config model as gu Plugin is shipped with following [Shareable Configs](http://eslint.org/docs/developer-guide/shareable-configs): - [angular](config/angular.js) - Set of rules for modern [Angular](https://angular.io) applications +- [angularjs](config/angularjs.js) - Set of rules for legacy [AngularJS](https://docs.angularjs.org) applications - [common](config/common.js) - Set of rules for common JavaScript applications - [electron](config/electron.js) - Set of rules for Electron applications - [node](config/node.js) - Set of rules for Node.js applications @@ -67,6 +68,9 @@ We also implemented several [custom rules](./lib/rules) where we did not find su | [no-new-func](https://eslint.org/docs/rules/no-new-func) | Bans calling `new Function()` as it's similar to `eval()` and prone to code execution. | | [node/no-deprecated-api](https://github.com/mysticatea/eslint-plugin-node/blob/master/docs/rules/no-deprecated-api.md) | Bans usage of deprecated APIs in Node. | | [@microsoft/sdl/no-angular-bypass-sanitizer](./docs/rules/no-angular-bypass-sanitizer.md) | Calls to bypassSecurityTrustHtml, bypassSecurityTrustScript and similar methods bypass [DomSanitizer](https://angular.io/api/platform-browser/DomSanitizer#security-risk) in Angular and need to be reviewed. | +| [@microsoft/sdl/no-angularjs-bypass-sce](./docs/rules/no-angularjs-bypass-sce.md) | Calls to `$sceProvider.enabled(false)`, `$sceDelegate.trustAs()`, `$sce.trustAs()` and relevant shorthand methods (e.g. `trustAsHtml` or `trustAsJs`) bypass [Strict Contextual Escaping (SCE)](https://docs.angularjs.org/api/ng/service/$sce#strict-contextual-escaping) in AngularJS and need to be reviewed. | +| [@microsoft/sdl/no-angularjs-enable-svg](./docs/rules/no-angularjs-enable-svg.md) | Calls to [`$sanitizeProvider.enableSvg(true)`](https://docs.angularjs.org/api/ngSanitize/provider/$sanitizeProvider#enableSvg) increase attack surface of the application by enabling SVG support in AngularJS sanitizer and need to be reviewed. | +| [@microsoft/sdl/no-angularjs-sanitization-whitelist](./docs/rules/no-angularjs-sanitization-whitelist.md) | Calls to [`$compileProvider.aHrefSanitizationWhitelist`](https://docs.angularjs.org/api/ng/provider/$compileProvider#aHrefSanitizationWhitelist) or [`$compileProvider.imgSrcSanitizationWhitelist`](https://docs.angularjs.org/api/ng/provider/$compileProvider#imgSrcSanitizationWhitelist) configure whitelists in AngularJS sanitizer and need to be reviewed. | | [@microsoft/sdl/no-cookies](./docs/rules/no-cookies.md) | HTTP cookies are an old client-side storage mechanism with inherent risks and limitations. Use Web Storage, IndexedDB or other modern methods instead. | | [@microsoft/sdl/no-document-domain](./docs/rules/no-document-domain.md) | Writes to [`document.domain`](https://developer.mozilla.org/en-US/docs/Web/API/Document/domain) property must be reviewed to avoid bypass of [same-origin checks](https://developer.mozilla.org/en-US/docs/Web/Security/Same-origin_policy#Changing_origin). Usage of top level domains such as `azurewebsites.net` is strictly prohibited. | | [@microsoft/sdl/no-document-write](./docs/rules/no-document-write.md) | Calls to document.write or document.writeln manipulate DOM directly without any sanitization and should be avoided. Use document.createElement() or similar methods instead. | diff --git a/config/angularjs.js b/config/angularjs.js new file mode 100644 index 0000000..c85b518 --- /dev/null +++ b/config/angularjs.js @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +"use strict"; + +// Generates shareable config for legacy AngularJS (https://angularjs.org/) apps. +module.exports = (pluginSdl) => { + return [ + { + plugins: { + "@microsoft/sdl": pluginSdl + }, + rules: { + "@microsoft/sdl/no-angularjs-enable-svg": "error", + "@microsoft/sdl/no-angularjs-sanitization-whitelist": "error", + "@microsoft/sdl/no-angularjs-bypass-sce": "error" + } + } + ]; +}; diff --git a/docs/rules/no-angularjs-bypass-sce.md b/docs/rules/no-angularjs-bypass-sce.md new file mode 100644 index 0000000..b43206b --- /dev/null +++ b/docs/rules/no-angularjs-bypass-sce.md @@ -0,0 +1,7 @@ +# Do not bypass Strict Contextual Escaping (SCE) in AngularJS (no-angularjs-bypass-sce) + +Calls to `$sceProvider.enabled(false)`, `$sceDelegate.trustAs()`, `$sce.trustAs()` and relevant shorthand methods (e.g. `trustAsHtml` or `trustAsJs`) bypass [Strict Contextual Escaping (SCE)](https://docs.angularjs.org/api/ng/service/$sce#strict-contextual-escaping) in AngularJS and need to be reviewed. + +SCE should be bypassed only in very rare and justifiable cases after careful review so that the risk of introducing Cross-Site-Scripting (XSS) vulnerability is minimized. + +See [official documentation](https://docs.angularjs.org/api/ng/service/$sce#strict-contextual-escaping) for more details. diff --git a/docs/rules/no-angularjs-enable-svg.md b/docs/rules/no-angularjs-enable-svg.md new file mode 100644 index 0000000..a5f2ad9 --- /dev/null +++ b/docs/rules/no-angularjs-enable-svg.md @@ -0,0 +1,7 @@ +# Do not enable SVG support in AngularJS (no-angularjs-enable-svg) + +Calls to [`$sanitizeProvider.enableSvg(true)`](https://docs.angularjs.org/api/ngSanitize/provider/$sanitizeProvider#enableSvg) increase attack surface of the application by enabling SVG support in AngularJS sanitizer and need to be reviewed. + +SVG support should be enabled only in very rare and justifiable cases after careful review so that the risk of introducing Clickjacking vulnerability is minimized. + +See [official documentation](https://docs.angularjs.org/api/ngSanitize/provider/$sanitizeProvider#enableSvg) for more details about the issue. diff --git a/docs/rules/no-angularjs-sanitization-whitelist.md b/docs/rules/no-angularjs-sanitization-whitelist.md new file mode 100644 index 0000000..ccf151a --- /dev/null +++ b/docs/rules/no-angularjs-sanitization-whitelist.md @@ -0,0 +1,7 @@ +# Do not bypass Angular's built-in sanitization (no-angularjs-sanitization-whitelist) + +Calls to [$compileProvider.aHrefSanitizationWhitelist](https://docs.angularjs.org/api/ng/provider/$compileProvider#aHrefSanitizationWhitelist) or [$compileProvider.imgSrcSanitizationWhitelist](https://docs.angularjs.org/api/ng/provider/$compileProvider#imgSrcSanitizationWhitelist) configure whitelists in AngularJS sanitizer and need to be reviewed. + +Sanitization should be disabled only in very rare and justifiable cases after careful review so that the risk of introducing Cross-Site-Scripting (XSS) vulnerability is minimized. + +See [official documentation](https://docs.angularjs.org/api/ng/provider/$compileProvider#aHrefSanitizationWhitelist) for more details about the issue. diff --git a/lib/index.js b/lib/index.js index 07a16bc..0a46f7c 100644 --- a/lib/index.js +++ b/lib/index.js @@ -16,6 +16,9 @@ const plugin = { rules: { "no-angular-bypass-sanitizer": require("./rules/no-angular-bypass-sanitizer"), "no-angular-sanitization-trusted-urls": require("./rules/no-angular-sanitization-trusted-urls"), + "no-angularjs-bypass-sce": require("./rules/no-angularjs-bypass-sce"), + "no-angularjs-enable-svg": require("./rules/no-angularjs-enable-svg"), + "no-angularjs-sanitization-whitelist": require("./rules/no-angularjs-sanitization-whitelist"), "no-cookies": require("./rules/no-cookies"), "no-document-domain": require("./rules/no-document-domain"), "no-document-write": require("./rules/no-document-write"), @@ -34,6 +37,7 @@ const plugin = { }; plugin.configs["angular"] = require("../config/angular")(plugin); +plugin.configs["angularjs"] = require("../config/angularjs")(plugin); plugin.configs["common"] = require("../config/common")(plugin); plugin.configs["electron"] = require("../config/electron")(plugin); plugin.configs["node"] = require("../config/node")(plugin); @@ -42,6 +46,7 @@ plugin.configs["typescript"] = require("../config/react")(plugin); plugin.configs["required"] = [ ...plugin.configs["angular"], + ...plugin.configs["angularjs"], ...plugin.configs["common"], ...plugin.configs["electron"], ...plugin.configs["node"], diff --git a/lib/rules/no-angularjs-bypass-sce.js b/lib/rules/no-angularjs-bypass-sce.js new file mode 100644 index 0000000..74bad0a --- /dev/null +++ b/lib/rules/no-angularjs-bypass-sce.js @@ -0,0 +1,69 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** + * @fileoverview Rule to disallow bypassing Strict Contextual Escaping (SCE) in AngularJS + * @author Antonios Katopodis + */ + +"use strict"; + +module.exports = { + meta: { + type: "suggestion", + fixable: "code", + schema: [], + docs: { + category: "Security", + description: + "Calls to $sceProvider.enabled(false), $sceDelegate.trustAs(), $sce.trustAs() and relevant shorthand methods (e.g. trustAsHtml or trustAsJs) bypass Strict Contextual Escaping (SCE) in AngularJS and need to be reviewed.", + url: "https://github.com/microsoft/eslint-plugin-sdl/blob/master/docs/rules/no-angularjs-bypass-sce.md" + }, + messages: { + doNotBypass: "Do not bypass Strict Contextual Escaping (SCE) in AngularJS" + } + }, + create: function (context) { + function reportIt(node) { + context.report({ + node: node, + messageId: "doNotBypass" + }); + } + + return { + "CallExpression[arguments!=''][callee.object.name='$sceProvider'][callee.property.name='enabled']"( + node + ) { + // Known false positives + if ( + node.arguments == undefined || + node.arguments.length != 1 || + (node.arguments[0].type == "Literal" && /true|1/.test(node.arguments[0].value)) + ) { + return; + } + return reportIt(node); + }, + "CallExpression[arguments!=''][callee.object.name='$sceDelegate'][callee.property.name='trustAs']": + reportIt, + "CallExpression[arguments!=''][callee.object.name='$sce'][callee.property.name=/trustAs(Css|Html|Js|ResourceUrl|Url)?/]"( + node + ) { + // Known false positives + if ( + node.arguments && + node.arguments.length === 1 && + node.arguments[0].type === "Literal" && + node.arguments[0].value === "" + ) { + return; + } + + return reportIt(node); + } + }; + } +}; + +// TODO: Review https://docs.angularjs.org/api/ng/provider/$sceDelegateProvider#resourceUrlWhitelist and https://docs.angularjs.org/api/ng/provider/$sceDelegateProvider#resourceUrlBlacklist diff --git a/lib/rules/no-angularjs-enable-svg.js b/lib/rules/no-angularjs-enable-svg.js new file mode 100644 index 0000000..7405601 --- /dev/null +++ b/lib/rules/no-angularjs-enable-svg.js @@ -0,0 +1,48 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** + * @fileoverview Rule to disallow enabling SVG in AngularJS apps + * @author Antonios Katopodis + */ + +"use strict"; + +module.exports = { + meta: { + type: "suggestion", + fixable: "code", + schema: [], + docs: { + category: "Security", + description: + "Calls to $sanitizeProvider.enableSvg(true) increase attack surface of the application by enabling SVG support in AngularJS sanitizer and need to be reviewed.", + url: "https://github.com/microsoft/eslint-plugin-sdl/blob/master/docs/rules/no-angularjs-enable-svg.md" + }, + messages: { + doNotEnableSVG: "Do not enable SVG support in AngularJS" + } + }, + create: function (context) { + return { + "CallExpression[callee.object.name='$sanitizeProvider'][callee.property.name='enableSvg']"( + node + ) { + // Known false positives + if ( + (node.arguments != undefined && node.arguments.length != 1) || + (node.arguments[0].type == "Literal" && + (node.arguments[0].value == "false" || node.arguments[0].value == "0")) + ) { + return; + } + context.report({ + node: node, + messageId: "doNotEnableSVG" + }); + } + }; + } +}; + +// TODO: Add rules for $sanitizeProvider.addValidElements() and $sanitizeProvider.addValidAttrs() diff --git a/lib/rules/no-angularjs-sanitization-whitelist.js b/lib/rules/no-angularjs-sanitization-whitelist.js new file mode 100644 index 0000000..c92e1f4 --- /dev/null +++ b/lib/rules/no-angularjs-sanitization-whitelist.js @@ -0,0 +1,38 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** + * @fileoverview Rule to disallow modifying sanitization whitelist in AngularJS + * @author Antonios Katopodis + */ + +"use strict"; + +module.exports = { + meta: { + type: "suggestion", + fixable: "code", + schema: [], + docs: { + category: "Security", + description: + "Calls to [`$compileProvider.aHrefSanitizationWhitelist`](https://docs.angularjs.org/api/ng/provider/$compileProvider#aHrefSanitizationWhitelist) or [`$compileProvider.imgSrcSanitizationWhitelist`](https://docs.angularjs.org/api/ng/provider/$compileProvider#imgSrcSanitizationWhitelist) configure whitelists in AngularJS sanitizer and need to be reviewed.", + url: "https://github.com/microsoft/eslint-plugin-sdl/blob/master/docs/rules/no-angularjs-sanitization-whitelist.md" + }, + messages: { + noSanitizationWhitelist: "Do not modify sanitization whitelist in AngularJS" + } + }, + create: function (context) { + return { + "CallExpression[arguments!=''][callee.object.name='$compileProvider'][callee.property.name=/(aHref|imgSrc)SanitizationWhitelist/]"( + node + ) { + context.report({ + node: node, + messageId: "noSanitizationWhitelist" + }); + } + }; + } +}; diff --git a/tests/lib/rules/no-angularjs-bypass-sce.js b/tests/lib/rules/no-angularjs-bypass-sce.js new file mode 100644 index 0000000..4fbe281 --- /dev/null +++ b/tests/lib/rules/no-angularjs-bypass-sce.js @@ -0,0 +1,63 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +const path = require("path"); +const ruleId = path.parse(__filename).name; +const rule = require(path.join("../../../lib/rules/", ruleId)); +const RuleTester = require("eslint").RuleTester; +var ruleTester = new RuleTester(); + +ruleTester.run(ruleId, rule, { + valid: [ + "trustAsHtml()", + "$sce.trustAsHtml()", + "$sce.trustAsHtml('')", + "$sce.TrustAsHtml('XSS')", + "x.trustAsHtml('XSS')", + "$sceProvider.enabled()", + "$sceProvider.enabled(true)", + "$sceProvider.enabled(1)" + ], + invalid: [ + { + code: "$sceDelegate.trustAs($sce.HTML, 'XSS')", + errors: [{ messageId: "doNotBypass" }] + }, + { + code: "$sce.trustAs($sce.HTML, 'XSS')", + errors: [{ messageId: "doNotBypass" }] + }, + { + code: "$sce.trustAsCss('XSS')", + errors: [{ messageId: "doNotBypass" }] + }, + { + code: "$sce.trustAsHtml('XSS')", + errors: [{ messageId: "doNotBypass" }] + }, + { + code: "$sce.trustAsJs('XSS')", + errors: [{ messageId: "doNotBypass" }] + }, + { + code: "$sce.trustAsResourceUrl('XSS')", + errors: [{ messageId: "doNotBypass" }] + }, + { + code: "$sce.trustAsUrl('XSS')", + errors: [{ messageId: "doNotBypass" }] + }, + { + code: "$sceProvider.enabled(false)", + errors: [{ messageId: "doNotBypass" }] + }, + { + code: "$sceProvider.enabled(0)", + errors: [{ messageId: "doNotBypass" }] + }, + { + code: "$sceProvider.enabled(true != true)", + errors: [{ messageId: "doNotBypass" }] + } + ] +}); diff --git a/tests/lib/rules/no-angularjs-enable-svg.js b/tests/lib/rules/no-angularjs-enable-svg.js new file mode 100644 index 0000000..ffc2922 --- /dev/null +++ b/tests/lib/rules/no-angularjs-enable-svg.js @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +const path = require("path"); +const ruleId = path.parse(__filename).name; +const rule = require(path.join("../../../lib/rules/", ruleId)); +const RuleTester = require("eslint").RuleTester; +var ruleTester = new RuleTester(); + +ruleTester.run(ruleId, rule, { + valid: [ + "enableSvg()", + "enableSvg(true)", + "$sanitizeProvider.enableSvg()", + "$sanitizeProvider.enableSvg(false)", + "$sanitizeProvider.enableSvg(0)", + "$sanitizeProvider.EnableSvg(0)" + ], + invalid: [ + { + code: "$sanitizeProvider.enableSvg(true)", + errors: [{ messageId: "doNotEnableSVG" }] + }, + { + code: "$sanitizeProvider.enableSvg(1)", + errors: [{ messageId: "doNotEnableSVG" }] + } + ] +}); diff --git a/tests/lib/rules/no-angularjs-sanitization-whitelist.js b/tests/lib/rules/no-angularjs-sanitization-whitelist.js new file mode 100644 index 0000000..728d342 --- /dev/null +++ b/tests/lib/rules/no-angularjs-sanitization-whitelist.js @@ -0,0 +1,43 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +const path = require("path"); +const ruleId = path.parse(__filename).name; +const rule = require(path.join("../../../lib/rules/", ruleId)); +const RuleTester = require("eslint").RuleTester; +var ruleTester = new RuleTester(); + +ruleTester.run(ruleId, rule, { + valid: [ + "aHrefSanitizationWhitelist('.*')", + "x.aHrefSanitizationWhitelist('.*')", + "$compileProvider.aHrefSanitizationWhitelist()", + "$compileProvider.AHrefSanitizationWhitelist('.*')" + ], + invalid: [ + { + code: "$compileProvider.aHrefSanitizationWhitelist('.*');", + errors: [ + { + messageId: "noSanitizationWhitelist", + line: 1, + endLine: 1, + column: 1, + endColumn: 50 + } + ] + }, + { + code: "$compileProvider.imgSrcSanitizationWhitelist('.*');", + errors: [ + { + messageId: "noSanitizationWhitelist", + line: 1, + endLine: 1, + column: 1, + endColumn: 51 + } + ] + } + ] +});