From 8780a756dc21ba353c1015d4608fe046eeaefcb4 Mon Sep 17 00:00:00 2001 From: Weverton Guedes Date: Wed, 29 Jul 2026 20:03:56 +0200 Subject: [PATCH] Check unsafe method calls made through bind() The method rule looked at the callee of a CallExpression and treated another CallExpression as fine, so a sink reached through Function.prototype.bind was never checked: document.body.insertAdjacentHTML.bind(document.body)("afterend", evil) Rewrite such a call into the direct call it performs and check that one instead, the same way the SequenceExpression case builds a mock node. Arguments given to bind() are prepended to the ones of the eventual call, so partial application is checked too. The rewrite only happens when the bound function is an identifier, a member expression or another bind(), so that binding an expression the rule cannot name stays as quiet as calling it directly would be. A spread in the thisArg position hides how many arguments bind() prepends, so those calls are left alone. Fixes #115 --- lib/rules/method.js | 60 ++++++++++++++++++++++++++++- tests/rules/method.js | 89 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 1 deletion(-) diff --git a/lib/rules/method.js b/lib/rules/method.js index 45c5020..4533381 100644 --- a/lib/rules/method.js +++ b/lib/rules/method.js @@ -61,6 +61,35 @@ function checkImport(ruleHelper, importExpr) { ruleHelper.checkMethod(fakeCall); } +// A bound function is only worth re-checking when we can tell which +// function is being bound. Rewriting any other expression would report its +// receiver as an unsupported callee, or duplicate a diagnostic that another +// visitor already emits for it. +const BINDABLE_CALLEES = ["Identifier", "MemberExpression", "CallExpression"]; + +/** + * Returns the function being bound by an expression that looks like a call to + * `Function.prototype.bind`, i.e., `foo.bar` for the `foo.bar.bind(foo)` part + * of `foo.bar.bind(foo)(baz)`. + * + * @param {object} callExpr The CallExpression to inspect + * @returns {object|undefined} The bound function, if we can name one + */ +function getBoundFunction(callExpr) { + const callee = callExpr.callee; + if ( + callee.type !== "MemberExpression" || + callee.computed || + callee.property.type !== "Identifier" || + callee.property.name !== "bind" + ) { + return undefined; + } + return BINDABLE_CALLEES.includes(callee.object.type) + ? callee.object + : undefined; +} + /** * Run ruleHelper.checkMethod for all but irrelevant callees (FunctionExpression, etc.) * @@ -129,6 +158,36 @@ function checkCallExpression(ruleHelper, callExpr, node) { break; } + case "CallExpression": { + // Calling the result of a call is only interesting when that call + // is a bind(): `foo.insertAdjacentHTML.bind(foo)(pos, bar)` runs + // the very same method as `foo.insertAdjacentHTML(pos, bar)`. + // So, we create a new mock CallExpression that calls the bound + // function directly and check that one instead. Issue #115. + const bound = getBoundFunction(node); + if (!bound) { + break; + } + + // bind() prepends its own arguments, all but `thisArg`, to the + // ones of the eventual call. A SpreadElement in the `thisArg` + // position may well stand for prepended arguments too and we + // cannot tell how many, so we leave those calls alone. + const [thisArg, ...prependedArguments] = node.arguments; + if (thisArg && thisArg.type === "SpreadElement") { + break; + } + + const newCallExpr = Object.assign({}, callExpr); + newCallExpr.callee = bound; + newCallExpr.arguments = [ + ...prependedArguments, + ...callExpr.arguments, + ]; + checkCallExpression(ruleHelper, newCallExpr, bound); + break; + } + case "TSAsExpression": break; @@ -138,7 +197,6 @@ function checkCallExpression(ruleHelper, callExpr, node) { case "ArrowFunctionExpression": case "FunctionExpression": case "Super": - case "CallExpression": case "ThisExpression": case "NewExpression": case "TSTypeAssertion": diff --git a/tests/rules/method.js b/tests/rules/method.js index 4d62e4a..2ac3693 100644 --- a/tests/rules/method.js +++ b/tests/rules/method.js @@ -408,6 +408,35 @@ eslintTester.run("method", rule, { // # 232: disallow setHTMLUnsafe, but OK with static string. code: "foo.setHTMLUnsafe('static string')", }, + { + // #115: `baz` is called on the bound function, and is not a sink. + code: "foo.bind(bar).baz()", + }, + { + code: 'document.body.insertAdjacentHTML.bind(document.body)("afterend", "harmless")', + }, + { + // #115: bind() may also provide the arguments. + code: 'document.body.insertAdjacentHTML.bind(document.body, "afterend", "harmless")()', + }, + { + // #115: the result of a call that is not a bind() stays unknown. + code: "getInserter()('afterend', evil)", + }, + { + // #115: a spread `thisArg` may carry prepended arguments as well, + // so we cannot tell where `evil` ends up. Like #214, we allow it. + code: "document.body.insertAdjacentHTML.bind(...l)('afterend', evil);", + ...ECMA_VERSION_2020_ONLY_OPTIONS, + }, + { + // #115: binding a callee we cannot name must stay quiet, rather + // than report it as an unsupported callee. + code: "(class {}).bind(null)()", + }, + { + code: "({ f: 1 }).bind(null)()", + }, ], // Examples of code that should trigger the rule @@ -967,5 +996,65 @@ eslintTester.run("method", rule, { }, ], }, + { + // #115: bound calls are checked like direct calls. + code: 'document.body.insertAdjacentHTML.bind(document.body)("afterend", foo)', + errors: [ + { + message: + /Unsafe call to document.body.insertAdjacentHTML for argument 1/, + }, + ], + }, + { + // #115: bind() may also provide the arguments. + code: 'document.body.insertAdjacentHTML.bind(document.body, "afterend")(foo)', + errors: [ + { + message: + /Unsafe call to document.body.insertAdjacentHTML for argument 1/, + }, + ], + }, + { + // #115: objectMatches still applies to the bound method. + code: "document.write.bind(document)(foo)", + errors: [ + { + message: /Unsafe call to document.write for argument 0/, + }, + ], + }, + { + // #115: binding an already bound function. + code: 'document.body.insertAdjacentHTML.bind(a).bind(b)("afterend", foo)', + errors: [ + { + message: + /Unsafe call to document.body.insertAdjacentHTML for argument 1/, + }, + ], + }, + { + // #115: a trailing spread does not hide the arguments before it. + code: "document.body.insertAdjacentHTML.bind(document.body, 'afterend', foo, ...rest)()", + errors: [ + { + message: + /Unsafe call to document.body.insertAdjacentHTML for argument 1/, + }, + ], + ...ECMA_VERSION_2020_ONLY_OPTIONS, + }, + { + // #115: binding an already reported call reports it just once. + code: "(document.body.insertAdjacentHTML`afterend${foo}`).bind(null)()", + errors: [ + { + message: + /Unsafe call to document.body.insertAdjacentHTML for argument 1/, + }, + ], + }, ], });