Vulnerable Regular Expression

[cristianstaicu]

The following regular expression used for parsing the cookie is vulnerable to ReDoS:

/^(([^=;]+))\s*=\s*([^\n\r\0]*)/

The slowdown is moderately low: for 50.000 characters around 2.5 seconds matching time. However, I would still suggest one of the following:

• remove the regex,
• limit the number of characters that can be matched by the repetition,
• limit the input size.
  I noticed there is another bug report regarding the correctness of this regular expression.

If needed, I can provide an actual example showing the slowdown.

[adamwdennis]

Nice find. An actual example will be helpful in verification of a fix.

[cristianstaicu]

Sure, here it is:

function genstr(len, chr) {
    var result = "";
    for (i=0; i<=len; i++) {
        result = result + chr;
    }
    return result;
}

var start = process.hrtime();
var tough = require('tough-cookie');
var str =  "x" + genstr(50000, ' ') + "x"; 
var Cookie = tough.Cookie;
var cookie = Cookie.parse(str);
var end = process.hrtime(start);

console.info("Execution time (hr): %ds %dms", end[0], end[1] / 1000000);

[evilpacket]

@cristianstaicu remember to start your timer right before your parse operation (i.e. after your require and string generation) for accurate time measurements. Given that node limits header size to 80kb the dos is limited to about 7.3 seconds. At Node Security we consider anything over 1 second to be a valid issue.

[eddieajau]

Just fyi, nsp has this on their radar so anyone with nsp in their CI build pipeline will now be experiencing failing builds.

[gemal]

https://nodesecurity.io/advisories/525

[tkalfigo]

Indeed. Got the nsp error from our pre-commit hook. But is there a way to overcome it temporarily until a fix for tough-cookie is in place? I'd like to avoid of course disabling entirely the nsp check.

[Hellenic]

You could add an exception to your .npmrc for that

[subesokun]

@adamwdennis Please fix as our builds are failing and I don't want to whitelist the advisory

[gemal]

add the following to a .nsprc file:

{
  "exceptions": ["https://nodesecurity.io/advisories/525"]
}

[henrysachs]

Hey Guys,
do you think it will take long for an update of the package. Because i want to use this in a project of mine and would like this to count on a fix in a decent amount of time. If that sounds kinda harsh it shouldn't. :D

[grnd]

While we haven't heard from the maintainers, suggesting limiting the number of whitespaces in the key.
#94

[tizmagik]

Snyk is picking this up now too, https://snyk.io/vuln/npm:tough-cookie:20170905