Skip to content

Commit 688a99e

Browse files
authored
Merge commit from fork
* Bound total expansion length across comma alternatives `maxLength` was enforced in `combine`, the one place output grows, but not on the `values` array built to feed it. Each alternative in `{a,b,c,...}` is expanded by its own `expand_` call, so each received a full independent `maxLength` allowance, and the results were concatenated with no running total. With `A` alternatives, `values` could reach `A * maxLength` characters before `combine` ever got the chance to truncate it. A ~25 KB input with 400 alternatives grew `values` past the heap limit and crashed the process with an uncatchable out-of-memory error, bypassing the mitigation added in 5.0.8 for CVE-2026-14257. Track the running result count and character length while appending alternatives, and stop once either bound is reached. * Apply maxLength while generating sequences `expandSequence` was bounded by `max`, the result count, but never consulted `maxLength`. A padded element's width follows the input, so a wide pad made it generate `max` wide elements only for `combine` to discard nearly all of them. Memory stays flat here, since V8 represents those padded strings as cons-strings, which is likely why this path was not caught alongside the memory-exhaustion issue. The cost is time instead, proportional to `max * width`: a ~400 KB input blocked the event loop for over two minutes. Pass `maxLength` into the generation loop and stop once the sequence's own characters reach it. Output is byte-identical; only the discarded work is removed. The same input now returns in ~18 ms. * fix: don't count dropped empties against `max` Capping the intermediate `values` array at `max` entries counted alternatives that `combine` goes on to drop as empty, so `max` stopped bounding the number of *kept* results: `expand('{a,,b}', { max: 2 })` returned `['a']` where it used to return `['a', 'b']`. Skip those values rather than counting them. The cap itself stays - it is what bounds the array when the values are empty and so contribute no characters for `maxLength` to see.
1 parent c66e5f9 commit 688a99e

2 files changed

Lines changed: 117 additions & 3 deletions

File tree

src/index.ts

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ function expandSequence(
161161
body: string,
162162
isAlphaSequence: boolean,
163163
max: number,
164+
maxLength: number,
164165
): string[] {
165166
const n = body.split(/\.\./)
166167
const N: string[] = []
@@ -186,6 +187,7 @@ function expandSequence(
186187
}
187188
const pad = n.some(isPadded)
188189

190+
let length = 0
189191
for (let i = x; test(i, y) && N.length < max; i += incr) {
190192
let c
191193
if (isAlphaSequence) {
@@ -207,7 +209,9 @@ function expandSequence(
207209
}
208210
}
209211
}
212+
if (length + c.length > maxLength) break
210213
N.push(c)
214+
length += c.length
211215
}
212216
return N
213217
}
@@ -290,7 +294,7 @@ function expand_(
290294

291295
let values: string[]
292296
if (isSequence) {
293-
values = expandSequence(m.body, isAlphaSequence, max)
297+
values = expandSequence(m.body, isAlphaSequence, max, maxLength)
294298
} else {
295299
let n = parseCommaParts(m.body)
296300
if (n.length === 1 && n[0] !== undefined) {
@@ -314,9 +318,31 @@ function expand_(
314318
/* c8 ignore stop */
315319
}
316320

321+
// Values that `combine` is going to drop as empty produce no result, so
322+
// they must not count against `max` - otherwise `{a,,b}` with `max: 2`
323+
// would stop at `['a', '']` and yield one result instead of two. Skipping
324+
// them outright keeps `values` bounded while leaving `max` a bound on
325+
// *kept* results.
326+
let dropsEmpties = dropEmpties && !m.post.length && !pre
327+
for (let d = 0; dropsEmpties && d < acc.length; d++) {
328+
if (acc[d]) {
329+
dropsEmpties = false
330+
}
331+
}
332+
317333
values = []
318-
for (let j = 0; j < n.length; j++) {
319-
values.push.apply(values, expand_(n[j] as string, max, maxLength, false))
334+
let valuesLength = 0
335+
outer: for (let j = 0; j < n.length; j++) {
336+
const expanded = expand_(n[j] as string, max, maxLength, false)
337+
for (let k = 0; k < expanded.length; k++) {
338+
const v = expanded[k] as string
339+
if (dropsEmpties && !v) continue
340+
if (values.length >= max || valuesLength + v.length > maxLength) {
341+
break outer
342+
}
343+
values.push(v)
344+
valuesLength += v.length
345+
}
320346
}
321347
}
322348

test/index.js

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,3 +277,91 @@ t.test('maxLength option bounds output size', async t => {
277277
`Expected total length (${dollarLength}) to respect maxLength`,
278278
)
279279
})
280+
281+
// Bypass of CVE-2026-14257's mitigation: each comma-separated alternative
282+
// (`{alt,alt,...}`) is expanded independently, and `maxLength` only bounded
283+
// each alternative's own output, not the running total accumulated across
284+
// all of them. Many alternatives - each individually far under `maxLength` -
285+
// could still sum to an unbounded intermediate array before the final
286+
// `combine` call ever got a chance to truncate.
287+
t.test('total length across comma alternatives is bounded', async t => {
288+
const alt = '{1..5}'
289+
const str = '{' + Array(1000).fill(alt).join(',') + '}'
290+
const startTime = performance.now()
291+
const expanded = expand(str, { maxLength: 50 })
292+
const endTime = performance.now()
293+
294+
const totalLength = expanded.reduce((sum, s) => sum + s.length, 0)
295+
t.ok(
296+
totalLength <= 50,
297+
`Expected total length (${totalLength}) to respect maxLength`,
298+
)
299+
t.ok(expanded.length > 0, 'still returns a (truncated) result')
300+
t.ok(
301+
endTime - startTime < 500,
302+
`Expected time (${endTime - startTime}ms) to be less than 500ms`,
303+
)
304+
305+
// Regression case from the report: 400 alternatives, each individually
306+
// bounded by maxLength but unbounded in aggregate before the fix.
307+
const part = '{' + '0'.repeat(50) + '1..100000}'
308+
const bigStr = '{' + Array(400).fill(part).join(',') + '}'
309+
t.doesNotThrow(() => {
310+
const bigExpanded = expand(bigStr)
311+
const bigTotal = bigExpanded.reduce((sum, s) => sum + s.length, 0)
312+
t.ok(
313+
bigTotal <= 4_000_000,
314+
`Expected total length (${bigTotal}) to stay bounded`,
315+
)
316+
})
317+
})
318+
319+
// A padded sequence's element width follows the input, so generating all `max`
320+
// elements before `combine` could discard them cost time proportional to
321+
// `max * width` - a ~400KB input blocked the event loop for over two minutes.
322+
t.test('padded sequences respect maxLength while generating', async t => {
323+
const str = '{' + '0'.repeat(400_000) + '1..100000}'
324+
const startTime = performance.now()
325+
const expanded = expand(str)
326+
const elapsed = performance.now() - startTime
327+
328+
const totalLength = expanded.reduce((sum, s) => sum + s.length, 0)
329+
t.ok(
330+
totalLength <= 4_000_000,
331+
`Expected total length (${totalLength}) to stay bounded`,
332+
)
333+
t.ok(expanded.length > 0, 'still returns a (truncated) result')
334+
t.ok(
335+
elapsed < 2000,
336+
`Expected time (${elapsed}ms) to be less than 2000ms`,
337+
)
338+
339+
// Truncating early must not change results that fit within the bound.
340+
t.same(
341+
expand('{01..10}'),
342+
['01', '02', '03', '04', '05', '06', '07', '08', '09', '10'],
343+
'padded sequences under the bound are unaffected',
344+
)
345+
})
346+
347+
// Bounding the intermediate `values` array must not change what `max` counts:
348+
// alternatives that expand to nothing are dropped by `combine`, so they cost a
349+
// slot in `values` but never a result.
350+
t.test('max bounds the number of kept results', async t => {
351+
t.same(
352+
expand('{a,,b}', { max: 2 }),
353+
['a', 'b'],
354+
'dropped empty alternatives do not count against max',
355+
)
356+
t.same(
357+
expand('{a,,,b,c}', { max: 3 }),
358+
['a', 'b', 'c'],
359+
'consecutive empty alternatives do not count against max',
360+
)
361+
// Here the empties survive as `xy`, so they are results and do count.
362+
t.same(
363+
expand('x{a,,b}y', { max: 2 }),
364+
['xay', 'xy'],
365+
'kept empty alternatives still count against max',
366+
)
367+
})

0 commit comments

Comments
 (0)