eslint-plugin-no-unsanitized - npm Package Compare versions

Comparing version 3.1.5 to 3.2.0

docs/rules/customization.md

 # Customization
 
+## Variable Tracing
+
+The plugin allows a limit back-tracing of variables.
+This will be used to check code like here:
+```js
+const greeting_template = `<p>Hello World!</p>`;
+// ... lots of other code in between ...
+someElemenet.innerHTML = greeting_template;
+```
+
+Currently, backtracing will only allow const and let variables that contain string literals only.
+Further assignments to these variables will also be checked for validation.
+
+**Backtracing can be disabled by setting the boolean
+option `variableTracing` to `false`.**
+
+Both values are supported and tested in CI.
+
+## Customization Examples
 You can customize the way this rule works in various ways.
@@ -9,3 +28,3 @@ * Add to the list of properties or functions to be checked for potentially
 
-## Examples
+
 ### Disallow the `html` function by specifically checking input for the first function parameter
@@ -12,0 +31,0 @@ ```json

lib/ruleHelper.js

@@ -28,14 +28,15 @@ /**
      * Returns true if the expression contains allowed syntax, otherwise false.
-     * For Template Strings with interpolation (e.g. |`${foo}`|) and
-     * Binary Expressions (e.g. |foo+bar|), the function will look into the expression
-     * recursively.
-     * For testing and development, I recommend looking at example code and its syntax tree.
-     * Using the Esprima Demo, for example: http://esprima.org/demo/parse.html
      *
-     * @constructor
+     * The function will be called recursively for Template Strings with interpolation
+     * (e.g. `Hello ${name}`), Binary Expressions (e.g. |foo+bar|), and more.
+     *
      * @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.
+     * @param {Object} details Additional linter violation state information, in case this function was called
+     *                         recursively.
+     * @returns {boolean} Returns whether the expression is allowed.
+     *
      */
-    allowedExpression(expression, escapeObject) {
+    allowedExpression(expression, escapeObject, details) {
         if (!escapeObject) {
@@ -53,43 +54,49 @@ escapeObject = {};
 
-        /*  surely, someone could have an evil literal in there, but that"s malice
-        we can just check for unsafe coding practice, not outright malice
-        example literal "<script>eval(location.hash.slice(1)</script>"
-        (it"s the task of the tagger-function to be the gateway here.)
-        */
-        if (expression.type === "Literal") {
-            // we just assign a literal (e.g. a string, a number, a bool)
+        switch(expression.type) {
+        case"Literal":
+            /*  surely, someone could have an evil literal in there, but that"s malice
+            we can just check for unsafe coding practice, not outright malice
+            example literal "<script>eval(location.hash.slice(1)</script>"
+            (it"s the task of the tagger-function to be the gateway here.)
+            */
             allowed = true;
-        } else if (expression.type === "TemplateElement") {
+            break;
+        case "TemplateElement":
             // Raw text from a template
             allowed = true;
-        } else if (expression.type === "TemplateLiteral") {
-            allowed = true;
+            break;
+        case "TemplateLiteral":
 
-            // check for ${..} expressions
-            for (let e = 0; e < expression.expressions.length; e++) {
-                const templateExpression = expression.expressions[e];
-                if (!this.allowedExpression(templateExpression, escapeObject)) {
-                    allowed = false;
-                    break;
-                }
-            }
-        } else if (expression.type === "TaggedTemplateExpression") {
+            // check only the ${..} expressions
+            allowed = this.allowedExpression(expression.expressions, escapeObject, details);
+            break;
+        case "TaggedTemplateExpression":
             allowed = this.isAllowedCallExpression(expression.tag, escapeObject.taggedTemplates || VALID_ESCAPERS);
-        } else if (expression.type === "CallExpression") {
+            break;
+        case "CallExpression":
             allowed = this.isAllowedCallExpression(expression.callee, escapeObject.methods || VALID_UNWRAPPERS);
-        } else if (expression.type === "BinaryExpression") {
-            allowed = ((this.allowedExpression(expression.left, escapeObject))
-            && (this.allowedExpression(expression.right, escapeObject)));
-        } else if (expression.type === "TSAsExpression") {
+            break;
+        case "BinaryExpression":
+            allowed = ((this.allowedExpression(expression.left, escapeObject, details))
+                && (this.allowedExpression(expression.right, escapeObject, details)));
+            break;
+        case "TSAsExpression":
             // TSAsExpressions contain the raw javascript value in 'expression'
-            allowed = this.allowedExpression(expression.expression, escapeObject);
-        } else if (expression.type === "TypeCastExpression") {
-            allowed = this.allowedExpression(expression.expression, escapeObject);
-        } else if (Array.isArray(expression)) {
-            allowed = expression.every((e) => this.allowedExpression(e, escapeObject));
-        }
-        else {
-            // everything that doesn't match is unsafe:
+            allowed = this.allowedExpression(expression.expression, escapeObject, details);
+            break;
+        case "TypeCastExpression":
+            allowed = this.allowedExpression(expression.expression, escapeObject, details);
+            break;
+        case "Identifier":
+            allowed = this.isAllowedIdentifier(expression, escapeObject, details);
+            break;
+        default:
+
+            // everything that doesn't match is considered unsafe:
             allowed = false;
+            break;
         }
+        if (Array.isArray(expression)) {
+            allowed = expression.every((e) => this.allowedExpression(e, escapeObject, details));
+        }
         return allowed;
@@ -99,2 +106,109 @@ },
     /**
+     * Check if an identifier is allowed
+     * - only if variableTracing is enabled in the first place.
+     * - find its declarations and see if it's const or let
+     * - if so, allow if the declaring statement is an allowed expression
+     * - ensure that following assignments to that identifier are also allowed
+     *
+     * @param {Object} expression Identifier expression
+     * @param {Object} escapeObject contains keys "methods" and "taggedTemplates" which are arrays of strings
+     *                              of matching escaping function names.
+     * @param {Object} details Additional linter violation state information, in case this function was called
+     *                         recursively.
+     * @returns {boolean} Returns whether the Identifier is deemed safe.
+     */
+    isAllowedIdentifier(expression, escapeObject, details) {
+
+        // respect the custom config property `variableTracing`:
+        if (!this.ruleChecks["variableTracing"]) {
+            return false;
+        }
+
+        // find declared variables and see which are literals
+        const scope = this.context.getScope(expression);
+        const variableInfo = scope.set.get(expression.name);
+        let allowed = false;
+
+        // If we can't get info on the variable, we just can't allow it
+        if (!variableInfo ||
+            !variableInfo.defs ||
+            variableInfo.defs.length == 0 ||
+            !variableInfo.references ||
+            variableInfo.references.length == 0) {
+            // FIXME Fix/Adjust towards a helpful message here and update tests accordingly.
+            // details.message = `Variable ${expression.name} considered unsafe: variable initialization not found`;
+            return false;
+        }
+
+        // look if the var was defined as allowable
+        let definedAsAllowed = false;
+        for (const def of variableInfo.defs) {
+            if (def.node.type !== "VariableDeclarator") {
+                // identifier wasn't declared as a variable
+                // e.g., it shows up as a parameter to an
+                // ArrowFunctionExpression, FunctionDeclaration or FunctionExpression
+                const {line, column} = def.node.loc.start;
+                if ((def.node.type === "FunctionDeclaration") || (def.node.type == "ArrowFunctionExpression") || (def.node.type === "FunctionExpression"))
+                {
+                    details.message = `Variable '${expression.name}' declared as function parameter, which is considered unsafe. '${def.node.type}' at ${line}:${column}`;
+                } else {
+                    details.message = `Variable '${expression.name}' initialized with unknown declaration '${def.node.type}' at ${line}:${column}`;
+                }
+                definedAsAllowed = false;
+                break;
+            }
+            if ((def.kind !== "let") && (def.kind !== "const")) {
+                // We do not allow for identifiers declared with "var", as they can be overridden in a
+                // way that is hard for us to follow (e.g., assignments to globalThis[theirNameAsString]).
+                definedAsAllowed = false;
+                break;
+            }
+
+            // the `init` property carries the right-hand side of the variable definition:
+            const varInitAs = def.node.init;
+            if (!this.allowedExpression(varInitAs, escapeObject, details)) {
+                // if one variable definition is considered unsafe, all are.
+                // NB: order of definition is unclear. See issue #168.
+                if (!details.message) {
+                    const {line, column} = varInitAs.loc.start;
+                    details.message = `Variable '${expression.name}' initialized with unsafe value at ${line}:${column}`;
+                }
+                definedAsAllowed = false;
+                break;
+            }
+
+            // keep iterating through other definitions.
+            definedAsAllowed = true;
+        }
+        if (definedAsAllowed) {
+            // the variable was declared as a safe value (e.g., literal)
+            // now inspect writing references to that variable
+            let allWritingRefsAllowed = false;
+            for (const ref of variableInfo.references) {
+                // only look into writing references
+                if (ref.isWrite()) {
+                    const writeExpr = ref.writeExpr;
+
+                    // if one is unsafe we'll consider all unsafe.
+                    // this is because code occurring doesn't guarantee it being executed
+                    // due to dynamic behavior if-conditions and such
+                    if (!this.allowedExpression(writeExpr, escapeObject, details)) {
+                        if (!details.message) {
+                            const {line, column} = writeExpr.loc.start;
+                            details.message = `Variable '${expression.name}' reassigned with unsafe value at ${line}:${column}`;
+                        }
+                        allWritingRefsAllowed = false;
+                        break;
+                    }
+                    allWritingRefsAllowed = true;
+                }
+            }
+
+            // allow this variable, because all writing references to it were allowed.
+            allowed = allWritingRefsAllowed;
+        }
+        return allowed;
+    },
+
+    /**
      * Check if a callee is in the list allowed sanitizers
@@ -134,2 +248,4 @@ *
             break;
+        case "ConditionalExpression":
+        case "CallExpression":
         case "ArrowFunctionExpression":
@@ -181,4 +297,4 @@ methodName = "";
         // Allow methods named "import":
-        if (normalizedMethodCall.methodName == "import"
-            && node.callee && node.callee.type == "MemberExpression") {
+        if (normalizedMethodCall.methodName === "import"
+            && node.callee && node.callee.type === "MemberExpression") {
             return false;
@@ -229,2 +345,8 @@ }
 
+        // default to no backtracing.
+        ruleCheckOutput["variableTracing"] = false;
+        if ("variableTracing" in parentRuleChecks) {
+            ruleCheckOutput["variableTracing"] = !!parentRuleChecks["variableTracing"];
+        }
+
         // If we have defined child rules lets ignore default rules
@@ -267,4 +389,11 @@ Object.keys(childRuleChecks).forEach((ruleCheckKey) => {
                 const argument = node.arguments[propertyId];
+                const details = {};
                 if (this.shouldCheckMethodCall(node, ruleCheck.objectMatches)
-                    && !this.allowedExpression(argument, ruleCheck.escape)) {
+                    && !this.allowedExpression(argument, ruleCheck.escape, details)) {
+                    // Include the additional details if available (e.g. name of a disallowed variable
+                    // and the position of the expression that made it disallowed).
+                    if (details.message) {
+                        this.context.report(node, `Unsafe call to ${this.getCodeName(node.callee)} for argument ${propertyId} (${details.message})`);
+                        return;
+                    }
                     this.context.report(node, `Unsafe call to ${this.getCodeName(node.callee)} for argument ${propertyId}`);
@@ -284,3 +413,10 @@ }
             const ruleCheck = this.ruleChecks[node.left.property.name];
-            if (!this.allowedExpression(node.right, ruleCheck.escape)) {
+            const details = {};
+            if (!this.allowedExpression(node.right, ruleCheck.escape, details)) {
+                // Include the additional details if available (e.g. name of a disallowed variable
+                // and the position of the expression that made it disallowed).
+                if (details.message) {
+                    this.context.report(node, `Unsafe assignment to ${node.left.property.name} (${details.message})`);
+                    return;
+                }
                 this.context.report(node, `Unsafe assignment to ${node.left.property.name}`);
@@ -287,0 +423,0 @@ }

package.json

 {
     "name": "eslint-plugin-no-unsanitized",
     "description": "ESLint rule to disallow unsanitized code",
-    "version": "3.1.5",
+    "version": "3.2.0",
     "author": {
@@ -6,0 +6,0 @@ "name": "Frederik Braun et al."

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

59576

increased by13.44%

Lines of code

663

increased by23.23%

No dependency changes