Skip to content

Commit 5171e68

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 b25213d commit 5171e68

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
@@ -45,34 +45,54 @@ function unescapeBraces(str) {
4545
}
4646

4747

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

58-
if (!m)
59-
return str.split(',');
61+
// Walk the brace groups iteratively. Recursing on `post` once per group let a
62+
// chain of them exhaust the stack - the parsing-side counterpart to
63+
// the `expand` overflow fixed for CVE-2026-14257, and not something `max` or
64+
// `maxLength` can bound, since it happens before expansion.
65+
//
66+
// The part the next chunk continues
67+
var carry = '';
6068

61-
var pre = m.pre;
62-
var body = m.body;
63-
var post = m.post;
64-
var p = pre.split(',');
69+
for (;;) {
70+
var m = balanced('{', '}', str);
6571

66-
p[p.length-1] += '{' + body + '}';
67-
var postParts = parseCommaParts(post);
68-
if (post.length) {
69-
p[p.length-1] += postParts.shift();
70-
p.push.apply(p, postParts);
71-
}
72+
if (!m) {
73+
var tail = str.split(',');
74+
tail[0] = carry + tail[0];
75+
pushAll(parts, tail);
76+
return parts;
77+
}
78+
79+
var pre = m.pre;
80+
var body = m.body;
81+
var post = m.post;
82+
var p = pre.split(',');
7283

73-
parts.push.apply(parts, p);
84+
p[0] = carry + p[0];
85+
p[p.length-1] += '{' + body + '}';
7486

75-
return parts;
87+
if (!post.length) {
88+
pushAll(parts, p);
89+
return parts;
90+
}
91+
92+
carry = p.pop();
93+
pushAll(parts, p);
94+
str = post;
95+
}
7696
}
7797

7898
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)