Skip to content

Commit 6735c94

Browse files
juliangruberjuliangruber
authored
Merge commit from fork
* fix: make parseCommaParts iterative and avoid push.apply `parseCommaParts` recursed on the remainder of the string once per brace group, so a chain of groups nested inside a brace set exhausted the native stack at ~7,000 groups (~29KB of input): expand('{' + '{a},'.repeat(7000) + 'b}') // RangeError: Maximum call stack size exceeded This is the parsing-side counterpart to the `expand_` overflow fixed for CVE-2026-14257. That fix documented a constant-stack-depth guarantee, but only `expand_` was made iterative, so putting the same chain inside a brace group routed parsing through the recursion that was left in place. Neither `max` nor `maxLength` could bound it: the crash happens while parsing, before anything is expanded, and the payload produces one result per group, so output size grows linearly and is never the limiter. Rewrite the function as a loop that carries the partial part across chunks. Separately, `push.apply(target, items)` passes one argument per element, so a single large array overflows the stack with no recursion at all - this input reaches a recursion depth of exactly one: expand('{{x},' + 'a,'.repeat(125000) + 'b}') // RangeError: Maximum call stack size exceeded Append element by element via `pushAll` instead. The leading `if (!str) return ['']` guard is dropped: it is unreachable from the sole call site (`m.body` always contains a comma there), and the loop returns `['']` for the empty string on its own. Equivalence with the previous implementation was checked by differential testing against 5.0.9 - exhaustive over every string of `{`, `}`, `,` and `a` up to length 8, plus 300k random inputs with and without `max` / `maxLength` - 387,381 cases, zero mismatches. * review: trim comments
1 parent 4e70465 commit 6735c94

2 files changed

Lines changed: 64 additions & 18 deletions

File tree

‎src/index.ts‎

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -52,36 +52,53 @@ function unescapeBraces(str: string) {
5252
.replace(escPeriodPattern, '.')
5353
}
5454

55+
// Like `target.push(...items)` but doesn't overflow the stack
56+
function pushAll(target: string[], items: string[]) {
57+
for (let i = 0; i < items.length; i++) {
58+
target.push(items[i] as string)
59+
}
60+
}
61+
5562
/**
5663
* Basically just str.split(","), but handling cases
5764
* where we have nested braced sections, which should be
5865
* treated as individual members, like {a,{b,c},d}
5966
*/
6067
function parseCommaParts(str: string) {
61-
if (!str) {
62-
return ['']
63-
}
64-
6568
const parts: string[] = []
66-
const m = balanced('{', '}', str)
6769

68-
if (!m) {
69-
return str.split(',')
70-
}
70+
// Walk the brace groups iteratively. Recursing on `post` once per group let a
71+
// chain of them exhaust the stack - the parsing-side counterpart to
72+
// the `expand_` overflow fixed for CVE-2026-14257, and not something `max` or
73+
// `maxLength` can bound, since it happens before expansion.
74+
//
75+
// The part the next chunk continues
76+
let carry = ''
7177

72-
const { pre, body, post } = m
73-
const p = pre.split(',')
78+
for (;;) {
79+
const m = balanced('{', '}', str)
7480

75-
p[p.length - 1] += '{' + body + '}'
76-
const postParts = parseCommaParts(post)
77-
if (post.length) {
78-
;(p[p.length - 1] as string) += postParts.shift()
79-
p.push.apply(p, postParts)
80-
}
81+
if (!m) {
82+
const tail = str.split(',')
83+
tail[0] = carry + (tail[0] as string)
84+
pushAll(parts, tail)
85+
return parts
86+
}
8187

82-
parts.push.apply(parts, p)
88+
const { pre, body, post } = m
89+
const p = pre.split(',')
90+
p[0] = carry + (p[0] as string)
91+
p[p.length - 1] += '{' + body + '}'
8392

84-
return parts
93+
if (!post.length) {
94+
pushAll(parts, p)
95+
return parts
96+
}
97+
98+
carry = p.pop() as string
99+
pushAll(parts, p)
100+
str = post
101+
}
85102
}
86103

87104
export type BraceExpansionOptions = {

‎test/index.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,35 @@ t.test('deep chaining does not overflow the stack', async t => {
258258
})
259259
})
260260

261+
// The same guarantee for the *parsing* side. `parseCommaParts` recursed on the
262+
// remainder of the string once per brace group, so chaining groups inside a
263+
// brace set exhausted the stack at ~7,000 groups (~29KB of input) even though
264+
// the identical chain outside one was already safe.
265+
t.test('deeply chained comma groups do not overflow the stack', async t => {
266+
const str = '{' + '{a},'.repeat(50_000) + 'b}'
267+
t.doesNotThrow(() => {
268+
const expanded = expand(str)
269+
t.ok(expanded.length > 0, 'still returns a result')
270+
})
271+
// The overflow happened while parsing, before anything was expanded, so
272+
// neither bound could prevent it - and neither is what keeps it safe now.
273+
t.doesNotThrow(
274+
() => expand(str, { max: 1, maxLength: 1 }),
275+
'still safe with both bounds set as low as they go',
276+
)
277+
})
278+
279+
// `push.apply(target, items)` passes one argument per element, so a single
280+
// large array overflowed the stack with no recursion at all - this input
281+
// reaches a recursion depth of one.
282+
t.test('a large comma set does not overflow the stack', async t => {
283+
const str = '{{x},' + 'a,'.repeat(200_000) + 'b}'
284+
t.doesNotThrow(() => {
285+
const expanded = expand(str)
286+
t.ok(expanded.length > 0, 'still returns a (truncated) result')
287+
})
288+
})
289+
261290
t.test('maxLength option bounds output size', async t => {
262291
const str = '{a,b}'.repeat(1500)
263292
const expanded = expand(str, { maxLength: 100_000 })

0 commit comments

Comments
 (0)