Skip to content

Commit c9bd79d

Browse files
committed
Parse cookie-pair part without regexp
Specifically to avoid any more hidden ReDoS in those regexes. Seems to run tests in 6.9s vs 7.0s so might be a bit of a speed bonus on some platforms!
1 parent 12d4266 commit c9bd79d

File tree

1 file changed

+52
-33
lines changed

1 file changed

+52
-33
lines changed

lib/cookie.js

Lines changed: 52 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -48,25 +48,14 @@ var DATE_DELIM = /[\x09\x20-\x2F\x3B-\x40\x5B-\x60\x7B-\x7E]/;
4848

4949
// From RFC6265 S4.1.1
5050
// note that it excludes \x3B ";"
51-
var COOKIE_OCTET = /[\x21\x23-\x2B\x2D-\x3A\x3C-\x5B\x5D-\x7E]/;
52-
var COOKIE_OCTETS = new RegExp('^'+COOKIE_OCTET.source+'+$');
51+
var COOKIE_OCTETS = /^[\x21\x23-\x2B\x2D-\x3A\x3C-\x5B\x5D-\x7E]+$/;
5352

5453
var CONTROL_CHARS = /[\x00-\x1F]/;
5554

56-
// For COOKIE_PAIR and LOOSE_COOKIE_PAIR below, the number of spaces has been
57-
// restricted to 256 to side-step a ReDoS issue reported here:
58-
// https://github.com/salesforce/tough-cookie/issues/92
59-
60-
// Double quotes are part of the value (see: S4.1.1).
61-
// '\r', '\n' and '\0' should be treated as a terminator in the "relaxed" mode
62-
// (see: https://github.com/ChromiumWebApps/chromium/blob/b3d3b4da8bb94c1b2e061600df106d590fda3620/net/cookies/parsed_cookie.cc#L60)
63-
// '=' and ';' are attribute/values separators
64-
// (see: https://github.com/ChromiumWebApps/chromium/blob/b3d3b4da8bb94c1b2e061600df106d590fda3620/net/cookies/parsed_cookie.cc#L64)
65-
var COOKIE_PAIR = /^(([^=;]+))\s{0,256}=\s*([^\n\r\0]*)/;
66-
67-
// Used to parse non-RFC-compliant cookies like '=abc' when given the `loose`
68-
// option in Cookie.parse:
69-
var LOOSE_COOKIE_PAIR = /^((?:=)?([^=;]*)\s{0,256}=\s*)?([^\n\r\0]*)/;
55+
// From Chromium // '\r', '\n' and '\0' should be treated as a terminator in
56+
// the "relaxed" mode, see:
57+
// https://github.com/ChromiumWebApps/chromium/blob/b3d3b4da8bb94c1b2e061600df106d590fda3620/net/cookies/parsed_cookie.cc#L60
58+
var TERMINATORS = ['\n', '\r', '\0'];
7059

7160
// RFC6265 S4.1.1 defines path value as 'any CHAR except CTLs or ";"'
7261
// Note ';' is \x3B
@@ -321,32 +310,62 @@ function defaultPath(path) {
321310
return path.slice(0, rightSlash);
322311
}
323312

313+
function trimTerminator(str) {
314+
for (var t = 0; t < TERMINATORS.length; t++) {
315+
var terminatorIdx = str.indexOf(TERMINATORS[t]);
316+
if (terminatorIdx !== -1) {
317+
str = str.substr(0,terminatorIdx);
318+
}
319+
}
324320

325-
function parse(str, options) {
326-
if (!options || typeof options !== 'object') {
327-
options = {};
321+
return str;
322+
}
323+
324+
function parseCookiePair(cookiePair, looseMode) {
325+
cookiePair = trimTerminator(cookiePair);
326+
327+
var firstEq = cookiePair.indexOf('=');
328+
if (looseMode) {
329+
if (firstEq === 0) { // '=' is immediately at start
330+
cookiePair = cookiePair.substr(1);
331+
firstEq = cookiePair.indexOf('='); // might still need to split on '='
332+
}
333+
} else { // non-loose mode
334+
if (firstEq <= 0) { // no '=' or is at start
335+
return; // needs to have non-empty "cookie-name"
336+
}
328337
}
329-
str = str.trim();
330338

331-
// We use a regex to parse the "name-value-pair" part of S5.2
332-
var firstSemi = str.indexOf(';'); // S5.2 step 1
333-
var pairRe = options.loose ? LOOSE_COOKIE_PAIR : COOKIE_PAIR;
334-
var result = pairRe.exec(firstSemi === -1 ? str : str.substr(0,firstSemi));
339+
var cookieName, cookieValue;
340+
if (firstEq <= 0) {
341+
cookieName = "";
342+
cookieValue = cookiePair.trim();
343+
} else {
344+
cookieName = cookiePair.substr(0, firstEq).trim();
345+
cookieValue = cookiePair.substr(firstEq+1).trim();
346+
}
335347

336-
// Rx satisfies the "the name string is empty" and "lacks a %x3D ("=")"
337-
// constraints as well as trimming any whitespace.
338-
if (!result) {
348+
if (CONTROL_CHARS.test(cookieName) || CONTROL_CHARS.test(cookieValue)) {
339349
return;
340350
}
341351

342352
var c = new Cookie();
343-
if (result[1]) {
344-
c.key = result[2].trim();
345-
} else {
346-
c.key = '';
353+
c.key = cookieName;
354+
c.value = cookieValue;
355+
return c;
356+
}
357+
358+
function parse(str, options) {
359+
if (!options || typeof options !== 'object') {
360+
options = {};
347361
}
348-
c.value = result[3].trim();
349-
if (CONTROL_CHARS.test(c.key) || CONTROL_CHARS.test(c.value)) {
362+
str = str.trim();
363+
364+
// We use a regex to parse the "name-value-pair" part of S5.2
365+
var firstSemi = str.indexOf(';'); // S5.2 step 1
366+
var cookiePair = (firstSemi === -1) ? str : str.substr(0, firstSemi);
367+
var c = parseCookiePair(cookiePair, !!options.loose);
368+
if (!c) {
350369
return;
351370
}
352371

0 commit comments

Comments
 (0)