Skip to content

Commit 1e84ebb

Browse files
committed
fix: Catch stack overflow during node composition
1 parent 6b24090 commit 1e84ebb

File tree

4 files changed

+97
-85
lines changed

4 files changed

+97
-85
lines changed

docs/08_errors.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ To identify errors for special handling, you should primarily use `code` to diff
4343
| `MULTIPLE_DOCS` | A YAML stream may include multiple documents. If yours does, you'll need to use `parseAllDocuments()` to work with it. |
4444
| `MULTIPLE_TAGS` | A node is only allowed to have one tag. |
4545
| `NON_STRING_KEY` | With the `stringKeys` option, all mapping keys must be strings |
46+
| `RESOURCE_EXHAUSTION` | The input document has excessive nesting, leading to a stack overflow during parsing. |
4647
| `TAB_AS_INDENT` | Only spaces are allowed as indentation. |
4748
| `TAG_RESOLVE_FAILED` | Something went wrong when resolving a node's tag with the current schema. |
4849
| `UNEXPECTED_TOKEN` | A token was encountered in a place where it wasn't expected. |

src/compose/compose-node.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,26 +61,25 @@ export function composeNode(
6161
case 'block-map':
6262
case 'block-seq':
6363
case 'flow-collection':
64-
node = composeCollection(CN, ctx, token, props, onError)
65-
if (anchor) node.anchor = anchor.source.substring(1)
64+
try {
65+
node = composeCollection(CN, ctx, token, props, onError)
66+
if (anchor) node.anchor = anchor.source.substring(1)
67+
} catch (error) {
68+
// Almost certainly here due to a stack overflow
69+
const message = error instanceof Error ? error.message : String(error)
70+
onError(token, 'RESOURCE_EXHAUSTION', message)
71+
}
6672
break
6773
default: {
6874
const message =
6975
token.type === 'error'
7076
? token.message
7177
: `Unsupported token (type: ${token.type})`
7278
onError(token, 'UNEXPECTED_TOKEN', message)
73-
node = composeEmptyNode(
74-
ctx,
75-
token.offset,
76-
undefined,
77-
null,
78-
props,
79-
onError
80-
)
8179
isSrcToken = false
8280
}
8381
}
82+
node ??= composeEmptyNode(ctx, token.offset, undefined, null, props, onError)
8483
if (anchor && node.anchor === '')
8584
onError(anchor, 'BAD_ALIAS', 'Anchor cannot be an empty string')
8685
if (

src/errors.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export type ErrorCode =
1919
| 'MULTIPLE_DOCS'
2020
| 'MULTIPLE_TAGS'
2121
| 'NON_STRING_KEY'
22+
| 'RESOURCE_EXHAUSTION'
2223
| 'TAB_AS_INDENT'
2324
| 'TAG_RESOLVE_FAILED'
2425
| 'UNEXPECTED_TOKEN'

tests/doc/parse.ts

Lines changed: 86 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -453,98 +453,109 @@ describe('odd indentations', () => {
453453
})
454454
})
455455

456-
describe('Excessive entity expansion attacks', () => {
457-
const root = resolve(__dirname, '../artifacts/pr104')
458-
const src1 = readFileSync(resolve(root, 'case1.yml'), 'utf8')
459-
const src2 = readFileSync(resolve(root, 'case2.yml'), 'utf8')
460-
const srcB = readFileSync(resolve(root, 'billion-laughs.yml'), 'utf8')
461-
const srcQ = readFileSync(resolve(root, 'quadratic.yml'), 'utf8')
462-
463-
let origEmitWarning: typeof process.emitWarning
464-
beforeAll(() => {
465-
origEmitWarning = process.emitWarning
466-
})
467-
afterAll(() => {
468-
process.emitWarning = origEmitWarning
469-
})
470-
471-
describe('Limit count by default', () => {
472-
for (const [name, src] of [
473-
['js-yaml case 1', src1],
474-
['js-yaml case 2', src2],
475-
['billion laughs', srcB],
476-
['quadratic expansion', srcQ]
477-
]) {
478-
test(name, () => {
479-
process.emitWarning = jest.fn()
480-
expect(() => YAML.parse(src)).toThrow(/Excessive alias count/)
481-
})
456+
describe('Resource exhaustion attacks', () => {
457+
test('Excessive recursion', () => {
458+
const depth = 5000
459+
const src = '['.repeat(depth) + '1' + ']'.repeat(depth)
460+
const doc = YAML.parseDocument(src)
461+
for (const error of doc.errors) {
462+
expect(error).toMatchObject({ code: 'RESOURCE_EXHAUSTION' })
482463
}
483464
})
484465

485-
describe('Work sensibly even with disabled limits', () => {
486-
test('js-yaml case 1', () => {
487-
process.emitWarning = jest.fn()
488-
const obj = YAML.parse(src1, { maxAliasCount: -1 })
489-
expect(obj).toMatchObject({})
490-
const key = Object.keys(obj)[0]
491-
expect(key.length).toBeGreaterThan(2000)
492-
expect(key.length).toBeLessThan(8000)
493-
expect(process.emitWarning).toHaveBeenCalled()
494-
})
466+
describe('Excessive entity expansion attacks', () => {
467+
const root = resolve(__dirname, '../artifacts/pr104')
468+
const src1 = readFileSync(resolve(root, 'case1.yml'), 'utf8')
469+
const src2 = readFileSync(resolve(root, 'case2.yml'), 'utf8')
470+
const srcB = readFileSync(resolve(root, 'billion-laughs.yml'), 'utf8')
471+
const srcQ = readFileSync(resolve(root, 'quadratic.yml'), 'utf8')
495472

496-
test('js-yaml case 2', () => {
497-
const arr = YAML.parse(src2, { maxAliasCount: -1 })
498-
expect(arr).toHaveLength(2)
499-
const key = Object.keys(arr[1])[0]
500-
expect(key).toBe('*id057')
473+
let origEmitWarning: typeof process.emitWarning
474+
beforeAll(() => {
475+
origEmitWarning = process.emitWarning
501476
})
502-
503-
test('billion laughs', () => {
504-
const obj = YAML.parse(srcB, { maxAliasCount: -1 })
505-
expect(Object.keys(obj)).toHaveLength(9)
477+
afterAll(() => {
478+
process.emitWarning = origEmitWarning
506479
})
507480

508-
test('quadratic expansion', () => {
509-
const obj = YAML.parse(srcQ, { maxAliasCount: -1 })
510-
expect(Object.keys(obj)).toHaveLength(11)
481+
describe('Limit count by default', () => {
482+
for (const [name, src] of [
483+
['js-yaml case 1', src1],
484+
['js-yaml case 2', src2],
485+
['billion laughs', srcB],
486+
['quadratic expansion', srcQ]
487+
]) {
488+
test(name, () => {
489+
process.emitWarning = jest.fn()
490+
expect(() => YAML.parse(src)).toThrow(/Excessive alias count/)
491+
})
492+
}
511493
})
512-
})
513494

514-
describe('maxAliasCount limits', () => {
515-
const rows = [
516-
'a: &a [lol, lol, lol, lol, lol, lol, lol, lol, lol]',
517-
'b: &b [*a, *a, *a, *a, *a, *a, *a, *a, *a]',
518-
'c: &c [*b, *b, *b, *b]',
519-
'd: &d [*c, *c]',
520-
'e: [*d]'
521-
]
495+
describe('Work sensibly even with disabled limits', () => {
496+
test('js-yaml case 1', () => {
497+
process.emitWarning = jest.fn()
498+
const obj = YAML.parse(src1, { maxAliasCount: -1 })
499+
expect(obj).toMatchObject({})
500+
const key = Object.keys(obj)[0]
501+
expect(key.length).toBeGreaterThan(2000)
502+
expect(key.length).toBeLessThan(8000)
503+
expect(process.emitWarning).toHaveBeenCalled()
504+
})
522505

523-
test(`depth 0: maxAliasCount 1 passes`, () => {
524-
expect(() => YAML.parse(rows[0], { maxAliasCount: 1 })).not.toThrow()
525-
})
506+
test('js-yaml case 2', () => {
507+
const arr = YAML.parse(src2, { maxAliasCount: -1 })
508+
expect(arr).toHaveLength(2)
509+
const key = Object.keys(arr[1])[0]
510+
expect(key).toBe('*id057')
511+
})
512+
513+
test('billion laughs', () => {
514+
const obj = YAML.parse(srcB, { maxAliasCount: -1 })
515+
expect(Object.keys(obj)).toHaveLength(9)
516+
})
526517

527-
test(`depth 1: maxAliasCount 1 fails on first alias`, () => {
528-
const src = `${rows[0]}\nb: *a`
529-
expect(() => YAML.parse(src, { maxAliasCount: 1 })).toThrow()
518+
test('quadratic expansion', () => {
519+
const obj = YAML.parse(srcQ, { maxAliasCount: -1 })
520+
expect(Object.keys(obj)).toHaveLength(11)
521+
})
530522
})
531523

532-
const limits = [10, 50, 150, 300]
533-
for (let i = 0; i < 4; ++i) {
534-
const src = rows.slice(0, i + 2).join('\n')
524+
describe('maxAliasCount limits', () => {
525+
const rows = [
526+
'a: &a [lol, lol, lol, lol, lol, lol, lol, lol, lol]',
527+
'b: &b [*a, *a, *a, *a, *a, *a, *a, *a, *a]',
528+
'c: &c [*b, *b, *b, *b]',
529+
'd: &d [*c, *c]',
530+
'e: [*d]'
531+
]
535532

536-
test(`depth ${i + 1}: maxAliasCount ${limits[i] - 1} fails`, () => {
537-
expect(() =>
538-
YAML.parse(src, { maxAliasCount: limits[i] - 1 })
539-
).toThrow()
533+
test(`depth 0: maxAliasCount 1 passes`, () => {
534+
expect(() => YAML.parse(rows[0], { maxAliasCount: 1 })).not.toThrow()
540535
})
541536

542-
test(`depth ${i + 1}: maxAliasCount ${limits[i]} passes`, () => {
543-
expect(() =>
544-
YAML.parse(src, { maxAliasCount: limits[i] })
545-
).not.toThrow()
537+
test(`depth 1: maxAliasCount 1 fails on first alias`, () => {
538+
const src = `${rows[0]}\nb: *a`
539+
expect(() => YAML.parse(src, { maxAliasCount: 1 })).toThrow()
546540
})
547-
}
541+
542+
const limits = [10, 50, 150, 300]
543+
for (let i = 0; i < 4; ++i) {
544+
const src = rows.slice(0, i + 2).join('\n')
545+
546+
test(`depth ${i + 1}: maxAliasCount ${limits[i] - 1} fails`, () => {
547+
expect(() =>
548+
YAML.parse(src, { maxAliasCount: limits[i] - 1 })
549+
).toThrow()
550+
})
551+
552+
test(`depth ${i + 1}: maxAliasCount ${limits[i]} passes`, () => {
553+
expect(() =>
554+
YAML.parse(src, { maxAliasCount: limits[i] })
555+
).not.toThrow()
556+
})
557+
}
558+
})
548559
})
549560
})
550561

0 commit comments

Comments
 (0)