express@5.0.0-alpha.2 vulnerability - adding automated vulnerability checks

[YasharF]

I just saw on https://snyk.io/test/npm/express that express@5.0.0-alpha.2 depends on a npm module with a hi sev security vulnerability.

1. Can the expressjs/express repo get integration to be monitor for new disclosures and check pull requests that introduce vulnerabilities? https://snyk.io/docs/github/#how-to-integrate-github-to-test-and-monitor-your-repositories
2. Does anyone have an objection if we add a vulnerability badge to readme.md ?

Regular Expression Denial of Service

High severity Vulnerable module: negotiator Introduced through: accepts@1.2.13 Detailed paths and remediation

Introduced through: express@5.0.0-alpha.2 › accepts@1.2.13 › negotiator@0.5.3 Remediation: Upgrade to accepts@1.3.3. Overview

negotiator is an HTTP content negotiator for Node.js. Versions prior to 0.6.1 are vulnerable to Regular expression Denial of Service (ReDoS) attack when parsing "Accept-Language" http header.

An attacker can provide a long value in the Accept-Language header, which nearly matches the pattern being matched. This will cause the regular expression matching to take a long time, all the while occupying the thread and preventing it from processing other requests. By repeatedly sending multiple such requests, the attacker can make the server unavailable (a Denial of Service attack).

[wesleytodd]

@dougwilson This should be removed probably. @YasharF when reporting security issues it is generally considered bad practice to post it publicly before the project maintainers have a chance to fix it.

[YasharF]

https://snyk.io/test/npm/express is publicly disclosed already. The disclosure/publish date was 06/16/2016.

[dougwilson]

Can the expressjs/express repo get integration to be monitor for new disclosures and check pull requests that introduce vulnerabilities?

That would never have caught this issue.

Does anyone have an objection if we add a vulnerability badge to readme.md ?

Yes, we don't want it; there are a few discussions around the repository about just this if you want to search around for them.

I am very aware of the vulun in the 5.0 alpha, as I was involved in all the reporting & disclosing. An alpha should be released soon, but as an alpha, we don't make additional effort to address these out-of-bounds. If you are concerned about security, you need to use a stable version of Express.

[dougwilson]

Sorry, didn't really mean to close. Feel free to close it, but otherwise I'm going to leave this open until we actually release a new alpha.

[wesleytodd]

Are there docs on reporting security voulns for the project? I am guessing you are treating this differently because it is an alpha release? Because typically a better practice would be emailing the maintainers privately before opening a public issue on the repo, thus giving them a chance to get the release out before emailing the 1400 people who watch this repo?

[wesleytodd]

Ahh, I think I found them: https://github.com/expressjs/express/blob/master/Security.md

Maybe this is something that could use some attention? Also maybe add it into an issue template so people don't post outstanding voulns in issues?

[dougwilson]

Yea, that is the file. Calling something Security.md in the top-level is seen as the common style for doing this, and we even link to it in our README (https://github.com/expressjs/express#security-issues). It is unfortunate that sometimes someone will miss these and just open an issue anyway, but there is no way we can undo all the emails GitHub has since sent out to all the watchers of the repository...

[YasharF]

A good place to add a reference would be https://github.com/expressjs/express/blob/master/Contributing.md , because when I was creating this github issue I saw the banner "Please review the guidelines for contributing to this repository." and went and read the guidelines. I didn't see anything in there about security related issues though, hence I went ahead and created this.

[dougwilson]

@YasharF, that file has the following:

The only exception is security dislosures which should be sent privately.

[YasharF]

wow, somehow I missed it. sorry.

[dougwilson]

No worries, I'm not worried :)

[dougwilson]

A new Express 5.0 alpha has been released: use npm install express@5.0.0-alpha.3 to get it :)