From c9bd79dd358ec8bb7ea82bea328b2449168736fc Mon Sep 17 00:00:00 2001 From: Jeremy Stashewsky Date: Thu, 21 Sep 2017 18:53:05 -0700 Subject: [PATCH] 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! --- lib/cookie.js | 85 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 33 deletions(-) diff --git a/lib/cookie.js b/lib/cookie.js index ccf941f8..ad1ef3ee 100644 --- a/lib/cookie.js +++ b/lib/cookie.js @@ -48,25 +48,14 @@ var DATE_DELIM = /[\x09\x20-\x2F\x3B-\x40\x5B-\x60\x7B-\x7E]/; // From RFC6265 S4.1.1 // note that it excludes \x3B ";" -var COOKIE_OCTET = /[\x21\x23-\x2B\x2D-\x3A\x3C-\x5B\x5D-\x7E]/; -var COOKIE_OCTETS = new RegExp('^'+COOKIE_OCTET.source+'+$'); +var COOKIE_OCTETS = /^[\x21\x23-\x2B\x2D-\x3A\x3C-\x5B\x5D-\x7E]+$/; var CONTROL_CHARS = /[\x00-\x1F]/; -// For COOKIE_PAIR and LOOSE_COOKIE_PAIR below, the number of spaces has been -// restricted to 256 to side-step a ReDoS issue reported here: -// https://github.com/salesforce/tough-cookie/issues/92 - -// Double quotes are part of the value (see: S4.1.1). -// '\r', '\n' and '\0' should be treated as a terminator in the "relaxed" mode -// (see: https://github.com/ChromiumWebApps/chromium/blob/b3d3b4da8bb94c1b2e061600df106d590fda3620/net/cookies/parsed_cookie.cc#L60) -// '=' and ';' are attribute/values separators -// (see: https://github.com/ChromiumWebApps/chromium/blob/b3d3b4da8bb94c1b2e061600df106d590fda3620/net/cookies/parsed_cookie.cc#L64) -var COOKIE_PAIR = /^(([^=;]+))\s{0,256}=\s*([^\n\r\0]*)/; - -// Used to parse non-RFC-compliant cookies like '=abc' when given the `loose` -// option in Cookie.parse: -var LOOSE_COOKIE_PAIR = /^((?:=)?([^=;]*)\s{0,256}=\s*)?([^\n\r\0]*)/; +// From Chromium // '\r', '\n' and '\0' should be treated as a terminator in +// the "relaxed" mode, see: +// https://github.com/ChromiumWebApps/chromium/blob/b3d3b4da8bb94c1b2e061600df106d590fda3620/net/cookies/parsed_cookie.cc#L60 +var TERMINATORS = ['\n', '\r', '\0']; // RFC6265 S4.1.1 defines path value as 'any CHAR except CTLs or ";"' // Note ';' is \x3B @@ -321,32 +310,62 @@ function defaultPath(path) { return path.slice(0, rightSlash); } +function trimTerminator(str) { + for (var t = 0; t < TERMINATORS.length; t++) { + var terminatorIdx = str.indexOf(TERMINATORS[t]); + if (terminatorIdx !== -1) { + str = str.substr(0,terminatorIdx); + } + } -function parse(str, options) { - if (!options || typeof options !== 'object') { - options = {}; + return str; +} + +function parseCookiePair(cookiePair, looseMode) { + cookiePair = trimTerminator(cookiePair); + + var firstEq = cookiePair.indexOf('='); + if (looseMode) { + if (firstEq === 0) { // '=' is immediately at start + cookiePair = cookiePair.substr(1); + firstEq = cookiePair.indexOf('='); // might still need to split on '=' + } + } else { // non-loose mode + if (firstEq <= 0) { // no '=' or is at start + return; // needs to have non-empty "cookie-name" + } } - str = str.trim(); - // We use a regex to parse the "name-value-pair" part of S5.2 - var firstSemi = str.indexOf(';'); // S5.2 step 1 - var pairRe = options.loose ? LOOSE_COOKIE_PAIR : COOKIE_PAIR; - var result = pairRe.exec(firstSemi === -1 ? str : str.substr(0,firstSemi)); + var cookieName, cookieValue; + if (firstEq <= 0) { + cookieName = ""; + cookieValue = cookiePair.trim(); + } else { + cookieName = cookiePair.substr(0, firstEq).trim(); + cookieValue = cookiePair.substr(firstEq+1).trim(); + } - // Rx satisfies the "the name string is empty" and "lacks a %x3D ("=")" - // constraints as well as trimming any whitespace. - if (!result) { + if (CONTROL_CHARS.test(cookieName) || CONTROL_CHARS.test(cookieValue)) { return; } var c = new Cookie(); - if (result[1]) { - c.key = result[2].trim(); - } else { - c.key = ''; + c.key = cookieName; + c.value = cookieValue; + return c; +} + +function parse(str, options) { + if (!options || typeof options !== 'object') { + options = {}; } - c.value = result[3].trim(); - if (CONTROL_CHARS.test(c.key) || CONTROL_CHARS.test(c.value)) { + str = str.trim(); + + // We use a regex to parse the "name-value-pair" part of S5.2 + var firstSemi = str.indexOf(';'); // S5.2 step 1 + var cookiePair = (firstSemi === -1) ? str : str.substr(0, firstSemi); + var c = parseCookiePair(cookiePair, !!options.loose); + if (!c) { return; }