TypeError: Cannot read property 'type' of undefined in detect-child-process rule

[XERXES-MOUSAVI]

Description

I get TypeError while linter is detecting MemberExpression of detect-child-process rule. The problem occurs while it's checking the exec() method. I'm using Mongoose as ODM to work with MongoDB. As it references here, there is an exec() method on its Query prototype to execute what query or callback we expect. But it is namesake with child_process.exec() and linter throws TypeError on every line that Mongoose Query.exec() appears.

https://github.com/nodesecurity/eslint-plugin-security/blob/edd1ae27245b0b220cbfdab59c3aaa0d279fea3d/rules/detect-child-process.js#L33-L37

Possible Solution

As I checked node.parent.arguments, there is no arguments while linting Mongoose exec(). I tested multiple scenarios of child_process.exec() and Mongoose Query.exec(). So that I believe if we check the emptiness of the arguments, the problem will be solved.

if (
  node.parent &&
  node.parent.arguments &&
  node.parent.arguments.length > 0 &&
  node.parent.arguments[0].type !== 'Literal'
) {
    return context.report(node, 'Found child_process.exec() with non Literal first argument');
}

[jesusprubio]

Summary
TypeError in detect-child-process rule.

Still relevant?
Yes.

Next steps

• Verify the reported problem in the actual codebase.
• Verify the proposed solution is correct.
• Send a PR with the fix.

[jesusprubio]

Good catch! The extra check for non-empty array is correct. A simple example which also brokes the rule implementation:

const child_process = require('child_process');
child_process.exec();

Output:

Oops! Something went wrong! :(

ESLint: 8.11.0

TypeError: Cannot read properties of undefined (reading 'type')
Occurred while linting /home/baguira/lintTest/index.js:2
Rule: "security/detect-child-process"
    at MemberExpression (/home/baguira/lintTest/node_modules/eslint-plugin-security/rules/detect-child-process.js:34:87)
    at ruleErrorHandler (/home/baguira/lintTest/node_modules/eslint/lib/linter/linter.js:1114:28)
    at /home/baguira/lintTest/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/home/baguira/lintTest/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/home/baguira/lintTest/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (/home/baguira/lintTest/node_modules/eslint/lib/linter/node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (/home/baguira/lintTest/node_modules/eslint/lib/linter/node-event-generator.js:340:14)
    at CodePathAnalyzer.enterNode (/home/baguira/lintTest/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:795:23)
    at /home/baguira/lintTest/node_modules/eslint/lib/linter/linter.js:1145:32

@XERXES-MOUSAVI , thank you! would you like to write a PR?

[lingo]

See also this merge request for a similar bug.

#97

[nzakas]

Closed by #97