Skip to content

Commit 0bcbfc0

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 the published release of this line - 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 758fcd6 commit 0bcbfc0

2 files changed

Lines changed: 91 additions & 18 deletions

File tree

‎index.js‎

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,34 +46,54 @@ function unescapeBraces(str) {
4646
}
4747

4848

49+
// Like `target.push(...items)` but doesn't overflow the stack
50+
function pushAll(target, items) {
51+
for (var i = 0; i < items.length; i++) {
52+
target.push(items[i]);
53+
}
54+
}
55+
4956
// Basically just str.split(","), but handling cases
5057
// where we have nested braced sections, which should be
5158
// treated as individual members, like {a,{b,c},d}
5259
function parseCommaParts(str) {
53-
if (!str)
54-
return [''];
55-
5660
var parts = [];
57-
var m = balanced('{', '}', str);
5861

59-
if (!m)
60-
return str.split(',');
62+
// Walk the brace groups iteratively. Recursing on `post` once per group let a
63+
// chain of them exhaust the stack - the parsing-side counterpart to
64+
// the `expand` overflow fixed for CVE-2026-14257, and not something `max` or
65+
// `maxLength` can bound, since it happens before expansion.
66+
//
67+
// The part the next chunk continues
68+
var carry = '';
69+
70+
for (;;) {
71+
var m = balanced('{', '}', str);
72+
73+
if (!m) {
74+
var tail = str.split(',');
75+
tail[0] = carry + tail[0];
76+
pushAll(parts, tail);
77+
return parts;
78+
}
6179

62-
var pre = m.pre;
63-
var body = m.body;
64-
var post = m.post;
65-
var p = pre.split(',');
80+
var pre = m.pre;
81+
var body = m.body;
82+
var post = m.post;
83+
var p = pre.split(',');
6684

67-
p[p.length-1] += '{' + body + '}';
68-
var postParts = parseCommaParts(post);
69-
if (post.length) {
70-
p[p.length-1] += postParts.shift();
71-
p.push.apply(p, postParts);
72-
}
85+
p[0] = carry + p[0];
86+
p[p.length-1] += '{' + body + '}';
7387

74-
parts.push.apply(parts, p);
88+
if (!post.length) {
89+
pushAll(parts, p);
90+
return parts;
91+
}
7592

76-
return parts;
93+
carry = p.pop();
94+
pushAll(parts, p);
95+
str = post;
96+
}
7797
}
7898

7999
function expandTop(str, options) {

‎test/ghsa-6j4f-fj2g-mc7p.js‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
var test = require('tape');
2+
var expand = require('..');
3+
4+
// `parseCommaParts` recursed on the remainder of the string once per brace
5+
// group, so chaining groups inside a brace set exhausted the native stack at
6+
// ~7,000 groups (~29KB of input) - the parsing-side counterpart to the
7+
// `expand` overflow fixed for CVE-2026-14257. The identical chain *outside* a
8+
// brace set (`'{a,b}'.repeat(n)`) was already safe; routing it through
9+
// `parseCommaParts` was not.
10+
test('deeply chained comma groups do not overflow the stack', function (t) {
11+
var str = '{' + '{a},'.repeat(50000) + 'b}'
12+
t.doesNotThrow(function () {
13+
var expanded = expand(str)
14+
t.ok(expanded.length > 0, 'still returns a result')
15+
})
16+
17+
// The overflow happened while parsing, before anything was expanded, so
18+
// neither bound could prevent it - and neither is what keeps it safe now.
19+
t.doesNotThrow(
20+
function () { expand(str, { max: 1, maxLength: 1 }) },
21+
'still safe with both bounds set as low as they go'
22+
)
23+
24+
t.end();
25+
})
26+
27+
// `push.apply(target, items)` passes one argument per element, so a single
28+
// large array overflowed the stack with no recursion at all - this input
29+
// reaches a recursion depth of exactly one.
30+
test('a large comma set does not overflow the stack', function (t) {
31+
var str = '{{x},' + 'a,'.repeat(200000) + 'b}'
32+
t.doesNotThrow(function () {
33+
var expanded = expand(str)
34+
t.ok(expanded.length > 0, 'still returns a (truncated) result')
35+
})
36+
37+
t.end();
38+
})
39+
40+
// The rewrite must not change what the parser produces.
41+
test('nested comma groups still parse as before', function (t) {
42+
t.deepEqual(expand('{a,b}{c,d}'), ['ac', 'ad', 'bc', 'bd'])
43+
t.deepEqual(expand('x{{a,b}}y'), ['x{a}y', 'x{b}y'])
44+
t.deepEqual(expand('{a,{b,c},d}'), ['a', 'b', 'c', 'd'])
45+
t.deepEqual(expand('{a,{b,c}d,e}'), ['a', 'bd', 'cd', 'e'])
46+
t.deepEqual(expand('x{a,{b,c},d}y'), ['xay', 'xby', 'xcy', 'xdy'])
47+
t.deepEqual(expand('{a,,b}'), ['a', 'b'])
48+
t.deepEqual(expand('{,}'), [])
49+
t.deepEqual(expand('{}'), ['{}'])
50+
t.deepEqual(expand('{a,b'), ['{a,b'])
51+
52+
t.end();
53+
})

0 commit comments

Comments
 (0)