Skip to content

fix: avoid unbounded recursion in Set-Cookie attribute parser #5757

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mcollina merged 1 commit into from
Sep 2, 2026
+165 −149
Merged

fix: avoid unbounded recursion in Set-Cookie attribute parser #5757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
299 changes: 150 additions & 149 deletions lib/web/cookies/parse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,223 +92,224 @@ function parseSetCookie (header) {
* @param {Object.<string, unknown>} [cookieAttributeList={}]
*/
function parseUnparsedAttributes (unparsedAttributes, cookieAttributeList = {}) {
// 1. If the unparsed-attributes string is empty, skip the rest of
// these steps.
if (unparsedAttributes.length === 0) {
return cookieAttributeList
}
while (true) {
// 1. If the unparsed-attributes string is empty, skip the rest of
// these steps.
if (unparsedAttributes.length === 0) {
return cookieAttributeList
}

// 2. Discard the first character of the unparsed-attributes (which
// will be a %x3B (";") character).
assert(unparsedAttributes[0] === ';')
unparsedAttributes = unparsedAttributes.slice(1)
// 2. Discard the first character of the unparsed-attributes (which
// will be a %x3B (";") character).
assert(unparsedAttributes[0] === ';')
unparsedAttributes = unparsedAttributes.slice(1)

let cookieAv = ''
let cookieAv = ''

// 3. If the remaining unparsed-attributes contains a %x3B (";")
// character:
if (unparsedAttributes.includes(';')) {
// 3. If the remaining unparsed-attributes contains a %x3B (";")
// character:
if (unparsedAttributes.includes(';')) {
// 1. Consume the characters of the unparsed-attributes up to, but
// not including, the first %x3B (";") character.
cookieAv = collectASequenceOfCodePointsFast(
';',
unparsedAttributes,
{ position: 0 }
)
unparsedAttributes = unparsedAttributes.slice(cookieAv.length)
} else {
cookieAv = collectASequenceOfCodePointsFast(
';',
unparsedAttributes,
{ position: 0 }
)
unparsedAttributes = unparsedAttributes.slice(cookieAv.length)
} else {
// Otherwise:

// 1. Consume the remainder of the unparsed-attributes.
cookieAv = unparsedAttributes
unparsedAttributes = ''
}
// 1. Consume the remainder of the unparsed-attributes.
cookieAv = unparsedAttributes
unparsedAttributes = ''
}

// Let the cookie-av string be the characters consumed in this step.
// Let the cookie-av string be the characters consumed in this step.

let attributeName = ''
let attributeValue = ''
let attributeName = ''
let attributeValue = ''

// 4. If the cookie-av string contains a %x3D ("=") character:
if (cookieAv.includes('=')) {
// 4. If the cookie-av string contains a %x3D ("=") character:
if (cookieAv.includes('=')) {
// 1. The (possibly empty) attribute-name string consists of the
// characters up to, but not including, the first %x3D ("=")
// character, and the (possibly empty) attribute-value string
// consists of the characters after the first %x3D ("=")
// character.
const position = { position: 0 }

attributeName = collectASequenceOfCodePointsFast(
'=',
cookieAv,
position
)
attributeValue = cookieAv.slice(position.position + 1)
} else {
const position = { position: 0 }

attributeName = collectASequenceOfCodePointsFast(
'=',
cookieAv,
position
)
attributeValue = cookieAv.slice(position.position + 1)
} else {
// Otherwise:

// 1. The attribute-name string consists of the entire cookie-av
// string, and the attribute-value string is empty.
attributeName = cookieAv
}
// 1. The attribute-name string consists of the entire cookie-av
// string, and the attribute-value string is empty.
attributeName = cookieAv
}

// 5. Remove any leading or trailing WSP characters from the attribute-
// name string and the attribute-value string.
attributeName = attributeName.trim()
attributeValue = attributeValue.trim()
// 5. Remove any leading or trailing WSP characters from the attribute-
// name string and the attribute-value string.
attributeName = attributeName.trim()
attributeValue = attributeValue.trim()

// 6. If the attribute-value is longer than 1024 octets, ignore the
// cookie-av string and return to Step 1 of this algorithm.
if (attributeValue.length > maxAttributeValueSize) {
return parseUnparsedAttributes(unparsedAttributes, cookieAttributeList)
}
// 6. If the attribute-value is longer than 1024 octets, ignore the
// cookie-av string and return to Step 1 of this algorithm.
if (attributeValue.length > maxAttributeValueSize) {
continue
}

// 7. Process the attribute-name and attribute-value according to the
// requirements in the following subsections. (Notice that
// attributes with unrecognized attribute-names are ignored.)
const attributeNameLowercase = attributeName.toLowerCase()
// 7. Process the attribute-name and attribute-value according to the
// requirements in the following subsections. (Notice that
// attributes with unrecognized attribute-names are ignored.)
const attributeNameLowercase = attributeName.toLowerCase()

// https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis#section-5.4.1
// If the attribute-name case-insensitively matches the string
// "Expires", the user agent MUST process the cookie-av as follows.
if (attributeNameLowercase === 'expires') {
// https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis#section-5.4.1
// If the attribute-name case-insensitively matches the string
// "Expires", the user agent MUST process the cookie-av as follows.
if (attributeNameLowercase === 'expires') {
// 1. Let the expiry-time be the result of parsing the attribute-value
// as cookie-date (see Section 5.1.1).
const expiryTime = new Date(attributeValue)

// 2. If the attribute-value failed to parse as a cookie date, ignore
// the cookie-av.
if (!Number.isNaN(expiryTime.getTime())) {
cookieAttributeList.expires = expiryTime
}
} else if (attributeNameLowercase === 'max-age') {
const expiryTime = new Date(attributeValue)

// 2. If the attribute-value failed to parse as a cookie date, ignore
// the cookie-av.
if (!Number.isNaN(expiryTime.getTime())) {
cookieAttributeList.expires = expiryTime
}
} else if (attributeNameLowercase === 'max-age') {
// https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis#section-5.4.2
// If the attribute-name case-insensitively matches the string "Max-
// Age", the user agent MUST process the cookie-av as follows.

// 1. If the first character of the attribute-value is not a DIGIT or a
// "-" character, ignore the cookie-av.
const charCode = attributeValue.charCodeAt(0)
const startsWithDigit = charCode >= 48 && charCode <= 57
const startsWithSignedDigit = attributeValue[0] === '-' && attributeValue.length > 1

if (!startsWithDigit && !startsWithSignedDigit) {
return parseUnparsedAttributes(unparsedAttributes, cookieAttributeList)
}

// 2. If the remainder of attribute-value contains a non-DIGIT
// character, ignore the cookie-av.
if (/[^\d]/.test(attributeValue.slice(1))) {
return parseUnparsedAttributes(unparsedAttributes, cookieAttributeList)
}

// 3. Let delta-seconds be the attribute-value converted to an integer.
const deltaSeconds = Number(attributeValue)

// 4. Let cookie-age-limit be the maximum age of the cookie (which
// SHOULD be 400 days or less, see Section 4.1.2.2).

// 5. Set delta-seconds to the smaller of its present value and cookie-
// age-limit.
// deltaSeconds = Math.min(deltaSeconds * 1000, maxExpiresMs)

// 6. If delta-seconds is less than or equal to zero (0), let expiry-
// time be the earliest representable date and time. Otherwise, let
// the expiry-time be the current date and time plus delta-seconds
// seconds.
// const expiryTime = deltaSeconds <= 0 ? Date.now() : Date.now() + deltaSeconds

// 7. Append an attribute to the cookie-attribute-list with an
// attribute-name of Max-Age and an attribute-value of expiry-time.
cookieAttributeList.maxAge = deltaSeconds
} else if (attributeNameLowercase === 'domain') {
// 1. If the first character of the attribute-value is not a DIGIT or a
// "-" character, ignore the cookie-av.
const charCode = attributeValue.charCodeAt(0)
const startsWithDigit = charCode >= 48 && charCode <= 57
const startsWithSignedDigit = attributeValue[0] === '-' && attributeValue.length > 1

if (!startsWithDigit && !startsWithSignedDigit) {
continue
}

// 2. If the remainder of attribute-value contains a non-DIGIT
// character, ignore the cookie-av.
if (/[^\d]/.test(attributeValue.slice(1))) {
continue
}

// 3. Let delta-seconds be the attribute-value converted to an integer.
const deltaSeconds = Number(attributeValue)

// 4. Let cookie-age-limit be the maximum age of the cookie (which
// SHOULD be 400 days or less, see Section 4.1.2.2).

// 5. Set delta-seconds to the smaller of its present value and cookie-
// age-limit.
// deltaSeconds = Math.min(deltaSeconds * 1000, maxExpiresMs)

// 6. If delta-seconds is less than or equal to zero (0), let expiry-
// time be the earliest representable date and time. Otherwise, let
// the expiry-time be the current date and time plus delta-seconds
// seconds.
// const expiryTime = deltaSeconds <= 0 ? Date.now() : Date.now() + deltaSeconds

// 7. Append an attribute to the cookie-attribute-list with an
// attribute-name of Max-Age and an attribute-value of expiry-time.
cookieAttributeList.maxAge = deltaSeconds
} else if (attributeNameLowercase === 'domain') {
// https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis#section-5.4.3
// If the attribute-name case-insensitively matches the string "Domain",
// the user agent MUST process the cookie-av as follows.

// 1. Let cookie-domain be the attribute-value.
let cookieDomain = attributeValue
// 1. Let cookie-domain be the attribute-value.
let cookieDomain = attributeValue

// 2. If cookie-domain starts with %x2E ("."), let cookie-domain be
// cookie-domain without its leading %x2E (".").
if (cookieDomain[0] === '.') {
cookieDomain = cookieDomain.slice(1)
}
// 2. If cookie-domain starts with %x2E ("."), let cookie-domain be
// cookie-domain without its leading %x2E (".").
if (cookieDomain[0] === '.') {
cookieDomain = cookieDomain.slice(1)
}

// 3. Convert the cookie-domain to lower case.
cookieDomain = cookieDomain.toLowerCase()
// 3. Convert the cookie-domain to lower case.
cookieDomain = cookieDomain.toLowerCase()

// 4. Append an attribute to the cookie-attribute-list with an
// attribute-name of Domain and an attribute-value of cookie-domain.
cookieAttributeList.domain = cookieDomain
} else if (attributeNameLowercase === 'path') {
// 4. Append an attribute to the cookie-attribute-list with an
// attribute-name of Domain and an attribute-value of cookie-domain.
cookieAttributeList.domain = cookieDomain
} else if (attributeNameLowercase === 'path') {
// https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis#section-5.4.4
// If the attribute-name case-insensitively matches the string "Path",
// the user agent MUST process the cookie-av as follows.

// 1. If the attribute-value is empty or if the first character of the
// attribute-value is not %x2F ("/"):
let cookiePath = ''
if (attributeValue.length === 0 || attributeValue[0] !== '/') {
// 1. If the attribute-value is empty or if the first character of the
// attribute-value is not %x2F ("/"):
let cookiePath = ''
if (attributeValue.length === 0 || attributeValue[0] !== '/') {
// 1. Let cookie-path be the default-path.
cookiePath = '/'
} else {
cookiePath = '/'
} else {
// Otherwise:

// 1. Let cookie-path be the attribute-value.
cookiePath = attributeValue
}
// 1. Let cookie-path be the attribute-value.
cookiePath = attributeValue
}

// 2. Append an attribute to the cookie-attribute-list with an
// attribute-name of Path and an attribute-value of cookie-path.
cookieAttributeList.path = cookiePath
} else if (attributeNameLowercase === 'secure') {
// 2. Append an attribute to the cookie-attribute-list with an
// attribute-name of Path and an attribute-value of cookie-path.
cookieAttributeList.path = cookiePath
} else if (attributeNameLowercase === 'secure') {
// https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis#section-5.4.5
// If the attribute-name case-insensitively matches the string "Secure",
// the user agent MUST append an attribute to the cookie-attribute-list
// with an attribute-name of Secure and an empty attribute-value.

cookieAttributeList.secure = true
} else if (attributeNameLowercase === 'httponly') {
cookieAttributeList.secure = true
} else if (attributeNameLowercase === 'httponly') {
// https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis#section-5.4.6
// If the attribute-name case-insensitively matches the string
// "HttpOnly", the user agent MUST append an attribute to the cookie-
// attribute-list with an attribute-name of HttpOnly and an empty
// attribute-value.

cookieAttributeList.httpOnly = true
} else if (attributeNameLowercase === 'samesite') {
cookieAttributeList.httpOnly = true
} else if (attributeNameLowercase === 'samesite') {
// https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis#section-5.4.7
// If the attribute-name case-insensitively matches the string
// "SameSite", the user agent MUST process the cookie-av as follows:

const attributeValueLowercase = attributeValue.toLowerCase()
const attributeValueLowercase = attributeValue.toLowerCase()

// 1. If cookie-av's attribute-value is a case-insensitive match for
// "None", append an attribute to the cookie-attribute-list with an
// attribute-name of "SameSite" and an attribute-value of "None".
if (attributeValueLowercase === 'none') {
cookieAttributeList.sameSite = 'None'
} else if (attributeValueLowercase === 'strict') {
// 1. If cookie-av's attribute-value is a case-insensitive match for
// "None", append an attribute to the cookie-attribute-list with an
// attribute-name of "SameSite" and an attribute-value of "None".
if (attributeValueLowercase === 'none') {
cookieAttributeList.sameSite = 'None'
} else if (attributeValueLowercase === 'strict') {
// 2. If cookie-av's attribute-value is a case-insensitive match for
// "Strict", append an attribute to the cookie-attribute-list with
// an attribute-name of "SameSite" and an attribute-value of
// "Strict".
cookieAttributeList.sameSite = 'Strict'
} else if (attributeValueLowercase === 'lax') {
cookieAttributeList.sameSite = 'Strict'
} else if (attributeValueLowercase === 'lax') {
// 3. If cookie-av's attribute-value is a case-insensitive match for
// "Lax", append an attribute to the cookie-attribute-list with an
// attribute-name of "SameSite" and an attribute-value of "Lax".
cookieAttributeList.sameSite = 'Lax'
}
} else {
cookieAttributeList.unparsed ??= []
cookieAttributeList.sameSite = 'Lax'
}
} else {
cookieAttributeList.unparsed ??= []

cookieAttributeList.unparsed.push(`${attributeName}=${attributeValue}`)
}
cookieAttributeList.unparsed.push(`${attributeName}=${attributeValue}`)
}

// 8. Return to Step 1 of this algorithm.
return parseUnparsedAttributes(unparsedAttributes, cookieAttributeList)
}
}

module.exports = {
Expand Down
15 changes: 15 additions & 0 deletions test/cookie/cookies.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,21 @@ test('Set-Cookie parser', () => {
assert.deepEqual(getSetCookies(headers), [])
})

test('Set-Cookie parser does not recurse unboundedly on many empty attributes', () => {
// Regression test for GHSA-pf6r-83r6-f2x8: a header of the form
// `a=b;;;;...` previously recursed once per attribute and overflowed
// the stack. It must now parse iteratively without throwing.
const headers = new Headers()
headers.append('set-cookie', 'a=b' + ';'.repeat(5000))

const cookies = getSetCookies(headers)
assert.strictEqual(cookies.length, 1)
assert.strictEqual(cookies[0].name, 'a')
assert.strictEqual(cookies[0].value, 'b')
assert.strictEqual(cookies[0].unparsed.length, 5000)
assert.ok(cookies[0].unparsed.every((entry) => entry === '='))
})

test('Set-Cookie parser does not percent-decode cookie values', () => {
assert.deepEqual(
parseCookie(
Expand Down
Loading