New Config: Avoiding intrusive and dangerous dependencies

[brettz9]

Rule details

Check node_modules for common security issues generally undesirable even in dependencies

Related CVE

XXXX

Example code

// Rules in the config might include:

// 1. `no-global-assign`:

Object = null

// 2. `no-eval`:

eval('user code');

// Detecting the setting of globals with `no-restricted-syntax` against
// `AssignmentExpression[left.object.name="window"]`

window.abc = 5;

Participation

• [X] I am willing to submit a pull request to implement this rule.

Additional comments

Perhaps it is beyond the scope of eslint-plugin-security, but I think the ESLint ecosystem deserves a focused config or plugin config which can make this concept of unignoring node_modules (and applying a relevant set of rules) salient and convenient for developers.

Three particular challenges occur:

1. Successfully parsing unknown code. I have used Babel's parser to overcome this limitation, sometimes adding Babel plugins where required for a given node_modules.
2. Not every file in a project may be visited by an app. While I'd hope there may be a more professionally-reviewed apps that does this, unaware of any, I created es-file-traverse to look at an app entry point and spit out a list of files that get statically or dynamically imported or required in its import chain. I then supply this to the ESLint command line, and it seems to work mostly well.
3. It of course becomes increasingly difficult to detect sophisticated obfuscation patterns or distinguish them from harmless polyfills, for example, so the plug-in may not catch all malicious code, but it may at least detect accidental mistakes that can be reported back to the project owner and could perhaps be refined over time to catch more patterns.

[nzakas]

Perhaps it is beyond the scope of eslint-plugin-security, but I think the ESLint ecosystem deserves a focused config or plugin config which can make this concept of unignoring node_modules (and applying a relevant set of rules) salient and convenient for developers.

That might be the case, though I'm not sure if that fits with this plugin's purpose. On their own, using eval or reassigning globals aren't necessarily a security risk. Linting code you haven't written also seems like a potentially confusing operation.

So, at the moment I don't see a compelling reason why such a config should exist in this plugin vs. starting your own specifically for this purpose.

[brettz9]

Can code like eval be used safely (or at a worthwhile cost)? Yes. But for the most part, there is a reason these rules exist, and it can be helpful to know that such code is being used. One can also add such files or projects to an ignore list if one deems it to be an acceptable risk, or disable that particular rule.

Some rules would most likely be too stringent to impose on others (though projects may vary in the risk they want to take--some might want to lint against dependencies using Node APIs like file system access, while others merely against intrusive operations).

But in using it, I have not found it confusing. In fact, I've been able to report bugs which were gratefully received. And no, I do not just report the issues without gaining some understanding of the code base.

But if this is not the home, that's fine. Thanks for your time.