diff --git a/packages/comark/src/internal/parse/token-processor.ts b/packages/comark/src/internal/parse/token-processor.ts index 4bfbb388..85ebdd36 100644 --- a/packages/comark/src/internal/parse/token-processor.ts +++ b/packages/comark/src/internal/parse/token-processor.ts @@ -155,6 +155,9 @@ function processAttributes( return attrs } +// Upper bound for expanded highlight ranges in a fence info string. +const MAX_HIGHLIGHT_LINES = 1_000 + /** * Parse codeblock info string to extract language, highlights, filename, and meta * Example: "javascript {1-3} [filename.ts] meta=value" @@ -196,7 +199,9 @@ function parseCodeblockInfo(info: string): { const highlightsStr = highlightsMatch[1] remaining = remaining.slice(highlightsMatch[0].length).trim() - // Parse highlight ranges and individual numbers + // Parse highlight ranges and individual numbers. Range expansion is + // bounded — a fence like ```js {1-999999999} must not materialize a + // billion-entry array from 20 bytes of markdown. const highlights: number[] = [] const parts = highlightsStr.split(',') for (const part of parts) { @@ -204,15 +209,15 @@ function parseCodeblockInfo(info: string): { if (trimmed.includes('-')) { // Range like "1-3" const [start, end] = trimmed.split('-').map((s) => Number.parseInt(s.trim(), 10)) - if (!Number.isNaN(start) && !Number.isNaN(end)) { - for (let i = start; i <= end; i++) { + if (!Number.isNaN(start) && !Number.isNaN(end) && end - start <= MAX_HIGHLIGHT_LINES) { + for (let i = start; i <= end && highlights.length < MAX_HIGHLIGHT_LINES; i++) { highlights.push(i) } } } else { // Single number const num = Number.parseInt(trimmed, 10) - if (!Number.isNaN(num)) { + if (!Number.isNaN(num) && highlights.length < MAX_HIGHLIGHT_LINES) { highlights.push(num) } } @@ -771,10 +776,18 @@ export function processInlineTokens(tokens: any[], inHeading: boolean = false): return mergeAdjacentTextNodes(nodes) } +// Cap on the html_inline lookahead recursion: each non-void opening tag +// recurses into the following tokens while searching for its matching close, +// so a long run of unclosed nested tags (e.g. 10k ``) would otherwise +// overflow the call stack. Beyond the cap the raw tag text is kept, matching +// the unrecognized-tag fallback. Mirrors markdown-it's default maxNesting. +const MAX_INLINE_HTML_DEPTH = 100 + function processInlineToken( tokens: any[], startIndex: number, - inHeading: boolean = false + inHeading: boolean = false, + htmlDepth: number = 0 ): { node: Node | string | null; nextIndex: number } { const token = tokens[startIndex] @@ -807,6 +820,11 @@ function processInlineToken( return { node: [tagInfo.tag, tagInfo.attrs] as Node, nextIndex: startIndex + 1 } } + if (htmlDepth >= MAX_INLINE_HTML_DEPTH) { + // Nesting too deep — keep the raw text instead of recursing further + return { node: content || null, nextIndex: startIndex + 1 } + } + // Non-void opening tag — look ahead for the matching closing tag const children: Node[] = [] let j = startIndex + 1 @@ -820,7 +838,7 @@ function processInlineToken( break } } - const result = processInlineToken(tokens, j, inHeading) + const result = processInlineToken(tokens, j, inHeading, htmlDepth + 1) j = result.nextIndex if (result.node) { children.push(result.node as Node) @@ -855,7 +873,7 @@ function processInlineToken( } // Process other tokens - const result = processInlineToken(tokens, i, inHeading) + const result = processInlineToken(tokens, i, inHeading, htmlDepth) i = result.nextIndex if (result.node) { nodes.push(result.node as Node) @@ -916,7 +934,7 @@ function processInlineToken( } // Process child token - const result = processInlineToken(tokens, i, inHeading) + const result = processInlineToken(tokens, i, inHeading, htmlDepth) i = result.nextIndex if (result.node) { children.push(result.node as Node) diff --git a/packages/comark/src/internal/stringify/attributes.ts b/packages/comark/src/internal/stringify/attributes.ts index adc76c8f..6e85b423 100644 --- a/packages/comark/src/internal/stringify/attributes.ts +++ b/packages/comark/src/internal/stringify/attributes.ts @@ -166,8 +166,13 @@ export function comarkAttributes(attributes: Record) { return `#${value}` } if (key === 'class') { - return (value as string) + // The parser JSON-decodes `[...]`/`{...}` attribute values, so class + // can be an array/object here — normalize instead of crashing on + // value.split. + const classValue = Array.isArray(value) ? value.join(' ') : String(value) + return classValue .split(' ') + .filter(Boolean) .map((c) => `.${c}`) .join('') } diff --git a/packages/comark/src/internal/stringify/state.ts b/packages/comark/src/internal/stringify/state.ts index e44ecbfd..e0ac4584 100644 --- a/packages/comark/src/internal/stringify/state.ts +++ b/packages/comark/src/internal/stringify/state.ts @@ -244,21 +244,57 @@ function isAlphaNumeric(char: string | undefined): boolean { return char !== undefined && /[a-zA-Z0-9]/.test(char) } +/** + * Build the index of the next `needle` from every position in `source` + * (-1 when none). One backwards pass, so per-position lookups are O(1). + */ +function buildNextIndex(source: string, charCode: number): Int32Array { + const next = new Int32Array(source.length + 1) + let last = -1 + next[source.length] = -1 + for (let i = source.length - 1; i >= 0; i--) { + if (source.charCodeAt(i) === charCode) last = i + next[i] = last + } + return next +} + /** * Escape inline syntax characters. `_`, `<` and `&` only start a construct in * specific positions, so they are left alone otherwise to avoid mangling * ordinary prose like `snake_case`, `a < b` or `AT&T`. */ function escapeInline(text: string): string { + // Lazily-built next-`>` index: the tag-likeness check below must not + // rescan the remainder of the node for every `<` (quadratic on long runs + // of `<` without a closing `>`). + let nextGt: Int32Array | undefined + return text.replace(inlineSyntax, (char, offset: number, source: string) => { // `_` only opens emphasis at a word boundary — never between alphanumerics. if (char === '_' && isAlphaNumeric(source[offset - 1]) && isAlphaNumeric(source[offset + 1])) { return char } // `<` only starts raw HTML / an autolink when it looks like a tag, not when - // used as a comparison (e.g. `a < b`). - if (char === '<' && !/^<[a-zA-Z!?/][^>]*>/.test(source.slice(offset))) { - return char + // used as a comparison (e.g. `a < b`). Tag-like means `<` followed by a + // letter/`!`/`?`/`/` with a `>` somewhere after — same shape as the + // previous /^<[a-zA-Z!?/][^>]*>/ test, minus the rescan. + if (char === '<') { + const next = source.charCodeAt(offset + 1) + const tagStart = + (next >= 65 && next <= 90) /* A-Z */ || + (next >= 97 && next <= 122) /* a-z */ || + next === 33 /* ! */ || + next === 63 /* ? */ || + next === 47 /* / */ + if (!tagStart) { + return char + } + nextGt ??= buildNextIndex(source, 62 /* > */) + if (nextGt[offset + 1] === -1) { + return char + } + return `\\${char}` } // `&` only starts a character reference when it forms an entity. if (char === '&' && !/^&#?[a-zA-Z0-9]+;/.test(source.slice(offset))) { diff --git a/packages/comark/src/plugins/json-render.ts b/packages/comark/src/plugins/json-render.ts index 60ccb874..e383eef4 100644 --- a/packages/comark/src/plugins/json-render.ts +++ b/packages/comark/src/plugins/json-render.ts @@ -4,6 +4,13 @@ import { defineComarkPlugin } from '../parse.ts' import { textContent, visit } from '../utils/index.ts' import { parseYaml } from '../internal/yaml.ts' +// Budgets for spec expansion. Specs are author-controlled and elements can +// reference the same key many times (a DAG), so recursive materialization +// must be bounded — otherwise a ~1KB fence inflates into billions of AST +// nodes and exhausts the heap before any renderer or sanitizer runs. +const MAX_EXPANDED_NODES = 10_000 +const MAX_DEPTH = 100 + function jsonRenderToAst(jrt: Spec | UIElement) { if (!(jrt as Spec).root) { jrt = { @@ -15,16 +22,28 @@ function jsonRenderToAst(jrt: Spec | UIElement) { const tree = jrt as Spec const root = tree.elements[tree.root] - return jsonRenderElementToAst(root, tree.elements) + return jsonRenderElementToAst(root, tree.elements, 0, { nodes: 0 }) } -function jsonRenderElementToAst(element: UIElement, elements: Record): Node { +function jsonRenderElementToAst( + element: UIElement, + elements: Record, + depth: number, + budget: { nodes: number } +): Node { + if (depth > MAX_DEPTH || ++budget.nodes > MAX_EXPANDED_NODES) { + throw new Error('json-render spec exceeds the expansion budget') + } if (element.type === 'Text') { return String(element.props.content) } const children = element.children?.map((childName) => elements[childName]).filter(Boolean) || [] - return [element.type, element.props, ...children.map((child) => jsonRenderElementToAst(child, elements))] + return [ + element.type, + element.props, + ...children.map((child) => jsonRenderElementToAst(child, elements, depth + 1, budget)), + ] } interface JsonRenderConfig {} diff --git a/packages/comark/src/plugins/rangi/language-comark.ts b/packages/comark/src/plugins/rangi/language-comark.ts index 1ea8459c..fc738ffc 100644 --- a/packages/comark/src/plugins/rangi/language-comark.ts +++ b/packages/comark/src/plugins/rangi/language-comark.ts @@ -157,8 +157,11 @@ export const comarkLanguage: ShjLanguageDefinition = [ ], // Heading carrying attributes — `## Title{#slug .lead}` + // The title is `[^{\n]*` (never `.*?[ \t]*`): both `.*?` and `[ \t]*` can + // match spaces, which made the rule backtrack catastrophically on headings + // with long space runs and no `{...}` block. [ - RegExp(`^ {0,3}#{1,6}[ \\t]+.*?[ \\t]*${ATTRS}[ \\t]*$`, 'gm'), + RegExp(`^ {0,3}#{1,6}[ \\t]+[^\\n{]*${ATTRS}[ \\t]*$`, 'gm'), 'section', [[RegExp(`${ATTRS}[ \\t]*$`, 'g'), undefined, attributeRules]], ], diff --git a/packages/comark/src/plugins/task-list.ts b/packages/comark/src/plugins/task-list.ts index afa74283..4ff09e8d 100644 --- a/packages/comark/src/plugins/task-list.ts +++ b/packages/comark/src/plugins/task-list.ts @@ -65,35 +65,6 @@ function attrSet(token: MarkdownItToken, name: string, value: string) { } } -function findParentListItem(tokens: MarkdownItToken[], index: number): number { - // Look backwards for list_item_open - for (let i = index - 1; i >= 0; i--) { - if (tokens[i].type === 'list_item_open') { - return i - } - if (tokens[i].type === 'list_item_close') { - // We've gone past the current list item - return -1 - } - } - return -1 -} - -function findParentList(tokens: MarkdownItToken[], listItemIndex: number): number { - const targetLevel = tokens[listItemIndex].level - 1 - - // Look backwards for the list (ul/ol) that contains this list item - for (let i = listItemIndex - 1; i >= 0; i--) { - if ( - tokens[i].level === targetLevel && - (tokens[i].type === 'bullet_list_open' || tokens[i].type === 'ordered_list_open') - ) { - return i - } - } - return -1 -} - function markdownItTaskList(md: MarkdownIt, options?: TaskListOptions) { const disableCheckboxes = !(options?.enabled ?? false) @@ -101,35 +72,59 @@ function markdownItTaskList(md: MarkdownIt, options?: TaskListOptions) { md.core.ruler.before('inline', 'task-lists-mdc', (state: MarkdownItState) => { const tokens = state.tokens + // Track the currently-open list items (and the list containing each) in a + // single forward pass. Scanning backwards per inline token is O(n²) on + // documents without any list, so plain paragraphs became a DoS vector. + const openItems: number[] = [] + // Parallel to openItems: the bullet_list_open/ordered_list_open index that + // contains each item (-1 when the item somehow opened without a list). + const openItemLists: number[] = [] + const openLists: number[] = [] + for (let i = 0; i < tokens.length; i++) { const token = tokens[i] - // Look for list items that might contain task lists - if (token.type === 'inline' && token.content) { - const parentIdx = findParentListItem(tokens, i) - - if (parentIdx >= 0) { - // Check if content starts with task list marker - const match = token.content.match(/^(\[[ x]\])\s+/i) + if (token.type === 'bullet_list_open' || token.type === 'ordered_list_open') { + openLists.push(i) + continue + } + if (token.type === 'bullet_list_close' || token.type === 'ordered_list_close') { + openLists.pop() + continue + } + if (token.type === 'list_item_open') { + openItems.push(i) + openItemLists.push(openLists.length > 0 ? openLists[openLists.length - 1] : -1) + continue + } + if (token.type === 'list_item_close') { + openItems.pop() + openItemLists.pop() + continue + } - if (match) { - const isChecked = match[1].toLowerCase() === '[x]' + // Look for list items that might contain task lists + if (token.type === 'inline' && token.content && openItems.length > 0) { + // Check if content starts with task list marker + const match = token.content.match(/^(\[[ x]\])\s+/i) - // Mark the list item with task-list-item class - attrSet(tokens[parentIdx], 'class', 'task-list-item') + if (match) { + const isChecked = match[1].toLowerCase() === '[x]' - // Mark the parent list with contains-task-list class - const listIdx = findParentList(tokens, parentIdx) - if (listIdx >= 0) { - attrSet(tokens[listIdx], 'class', 'contains-task-list') - } + // Mark the list item with task-list-item class + attrSet(tokens[openItems[openItems.length - 1]], 'class', 'task-list-item') - // Replace the task marker with a placeholder that won't be processed by Comark - // We use a special format that we can detect later - // Keep one space after the placeholder to match expected output - const checkboxPlaceholder = `TASK_CHECKBOX_${isChecked ? 'CHECKED' : 'UNCHECKED'} ` - token.content = token.content.replace(/^\[[ x]\]\s+/i, checkboxPlaceholder) + // Mark the parent list with contains-task-list class + const listIdx = openItemLists[openItemLists.length - 1] + if (listIdx >= 0) { + attrSet(tokens[listIdx], 'class', 'contains-task-list') } + + // Replace the task marker with a placeholder that won't be processed by Comark + // We use a special format that we can detect later + // Keep one space after the placeholder to match expected output + const checkboxPlaceholder = `TASK_CHECKBOX_${isChecked ? 'CHECKED' : 'UNCHECKED'} ` + token.content = token.content.replace(/^\[[ x]\]\s+/i, checkboxPlaceholder) } } } diff --git a/packages/comark/test/dos.test.ts b/packages/comark/test/dos.test.ts new file mode 100644 index 00000000..542ae62e --- /dev/null +++ b/packages/comark/test/dos.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest' +import { parseMarkdown } from '../src/parse' +import { renderMarkdown } from '../src/render' + +describe('denial-of-service hardening', () => { + it('parses thousands of unclosed nested inline HTML tags without stack overflow', async () => { + const md = ''.repeat(10_000) + const doc = await parseMarkdown(md) + expect(doc.nodes.length).toBeGreaterThan(0) + }) + + it('clamps unbounded codeblock highlight ranges', async () => { + const doc = await parseMarkdown('```js {1-999999999}\nconst x = 1\n```') + const pre = doc.nodes[0] as [string, Record, ...unknown[]] + // Oversized ranges are dropped rather than expanded + expect((pre[1].highlights as number[] | undefined)?.length ?? 0).toBeLessThanOrEqual(1_000) + }) + + it('keeps highlight lines beyond the block length (documented behavior)', async () => { + const doc = await parseMarkdown('```js {1-3,50}\nconst x = 1\nconst y = 2\n```') + const pre = doc.nodes[0] as [string, Record, ...unknown[]] + expect(pre[1].highlights).toEqual([1, 2, 3, 50]) + }) + + it('serializes a non-string class attribute without throwing', async () => { + const doc = await parseMarkdown('Hi [x]{class=["a","b"]}') + await expect(renderMarkdown(doc)).resolves.toContain('.a.b') + }) + + it('parses large plain documents without the task-list quadratic scan', async () => { + // The default task-list rule used to scan all preceding tokens backwards + // for every inline token — O(n²) on documents that contain no list. + const md = Array.from({ length: 2_000 }, (_, i) => `Paragraph ${i} with some text.`).join('\n\n') + const doc = await parseMarkdown(md) + expect(doc.nodes.length).toBe(2_000) + }) + + it('still marks task lists after the linear rewrite', async () => { + const doc = await parseMarkdown('- [ ] todo\n- [x] done\n\n- outer\n - [x] nested\n') + const html = JSON.stringify(doc.nodes) + expect(html).toContain('task-list-item') + expect(html).toContain('contains-task-list') + }) + + it('escapes long runs of tag-like text without quadratic rescanning', async () => { + // escapeInline used to re-test /^<[a-zA-Z!?/][^>]*>/ against the whole + // remainder of the node for every `<` — O(n²) on runs like `` anywhere, so nothing is tag-like and the text stays untouched + expect(md.trim()).toBe(text) + }) +}) diff --git a/packages/comark/test/plugins/json-render.test.ts b/packages/comark/test/plugins/json-render.test.ts new file mode 100644 index 00000000..32723dfe --- /dev/null +++ b/packages/comark/test/plugins/json-render.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest' +import { parseMarkdown } from '../../src/parse' +import jsonRender from '../../src/plugins/json-render' + +describe('json-render plugin', () => { + it('expands a small spec into AST nodes', async () => { + const spec = JSON.stringify({ + root: 'card', + elements: { + card: { type: 'Card', props: { title: 'Hello' }, children: ['text'] }, + text: { type: 'Text', props: { content: 'World' } }, + }, + }) + const doc = await parseMarkdown(`\`\`\`json-render\n${spec}\n\`\`\``, { plugins: [jsonRender()] }) + const first = doc.nodes[0] as any + expect(first[0]).toBe('Card') + expect(first[2]).toBe('World') + }) + + it('bounds exponential expansion of DAG-shaped specs', async () => { + // Each element references the next one twice: a ~1KB spec would + // materialize 2^depth AST nodes without a budget (heap exhaustion). + const levels = 14 + const elements: Record = {} + for (let i = 0; i < levels; i++) { + elements[`e${i}`] = + i === levels - 1 + ? { type: 'Text', props: { content: 'leaf' } } + : { type: 'Stack', props: {}, children: [`e${i + 1}`, `e${i + 1}`] } + } + const spec = JSON.stringify({ root: 'e0', elements }) + const doc = await parseMarkdown(`\`\`\`json-render\n${spec}\n\`\`\``, { plugins: [jsonRender()] }) + const first = doc.nodes[0] as any + // The expansion budget tripped, the throw was caught, and the fence stays inert + expect(first[0]).toBe('pre') + expect(first[1].language).toBe('json-render') + }) +}) diff --git a/packages/comark/test/plugins/rangi.test.ts b/packages/comark/test/plugins/rangi.test.ts index eee1bf6e..49e56fc9 100644 --- a/packages/comark/test/plugins/rangi.test.ts +++ b/packages/comark/test/plugins/rangi.test.ts @@ -46,6 +46,16 @@ describe('tokenizeCode', () => { expect(tokens.map((t) => t.text).join('')).toBe('const x = 1') }) + it('does not catastrophically backtrack on headings without attributes', () => { + // The heading-with-attributes rule was `^ {0,3}#{1,6}[ \t]+.*?[ \t]*ATTRS[ \t]*$`: + // the lazy `.*?` and the greedy `[ \t]*` both matched spaces, so a heading + // followed by thousands of spaces and no `{...}` exploded combinatorially + // (~9s for 3000 spaces). + const payload = `# ${' '.repeat(3_000)}x` + const tokens = tokenizeCode(payload, 'md') + expect(tokens.map((t) => t.text).join('')).toBe(payload) + }) + it('resolves language aliases via rangi (typescript)', () => { const tokens = tokenizeCode('const n: number = 1', 'typescript') expect(tokens.some((t) => t.type === 'kwd')).toBe(true) diff --git a/test/bundle.test.ts b/test/bundle.test.ts index b553ccc9..569d381b 100644 --- a/test/bundle.test.ts +++ b/test/bundle.test.ts @@ -67,7 +67,7 @@ describe('package bundle size', { timeout: 60_000 }, () => { "@comark/react": "43.6k (74 files)", "@comark/svelte": "43.9k (82 files)", "@comark/vue": "60.5k (78 files)", - "comark": "405k (154 files)", + "comark": "409k (154 files)", } `) })