Consider adding eslint-plugin-security

[LinusU]

This seems to catch quite a few security related issues: https://github.com/nodesecurity/eslint-plugin-security

also see #1085

[feross]

Yes, this has been on my list of things to look at. Will consider for standard 12.

[alasdairhurst]

Given that standard is supposed to be no-config, having plugin security could be a pain out the box. There's a lot of stuff in it warning on the use of child_process, buffers and the like which are not bad, they can just be used in bad ways if you're not careful. most of the time will just get eslint ignored resulting in comment clutter.

[LinusU]

Yeah, we should evaluate each individual rule and decide which to turn on 👍

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[feross]

I just took a closer look at the rules available in eslint-plugin-security. While there's some good stuff in here, the false positive rate is waay too high on almost every rule for this to be a daily rule. This seems mostly geared towards security auditors and pen-testers who want to look at a big list of possibly unsafe code patterns to find the needle in the haystack.

Of all the rules, the only three that had a low enough error rate that we could plausibly enable them are:

detect-disable-mustache-escape
Detects object.escapeMarkup = false, which can be used with some template engines to disable escaping of HTML entities. This can lead to Cross-Site Scripting (XSS) vulnerabilities.

Could be useful but it's specific to one template language. Worth adding a whole new ESLint plugin to get this rule?

detect-no-csrf-before-method-override
Detects Express csrf middleware setup before method-override middleware. This can allow GET requests (which are not checked by csrf) to turn into POST requests later.

Again, could be useful in some codebases, but specific to express and two middlewares.

detect-possible-timing-attacks
Detects insecure comparisons (==, !=, !== and ===), which check input sequentially.

Detects comparisons of a variable named hash with something else using ===, which could lead to a timing attack. Triggered false positives in the webtorrent codebase since we use the variable name hash decently often and it has nothing to do with user passwords.

Overall, not too concerned about not getting these rules, so I'm going to close this issue. Feedback on this decision welcome.