Skip to content

fix(markdown) bound the link label and destination runs - #4544

Open
spokodev wants to merge 1 commit into
highlightjs:mainfrom
spokodev:fix/markdown-link-backtracking
Open

spokodev wants to merge 1 commit into
highlightjs:mainfrom
spokodev:fix/markdown-link-backtracking

Conversation

@spokodev

Copy link
Copy Markdown
Contributor

Changes

Each of the five LINK variants pairs an unbounded lazy label with an unbounded lazy destination:

begin: /\[.+?\]\([./?&#].*?\)/

On text that opens a link and never closes one, the engine grows the label one character at a time and, for every label length, grows the destination again — at every start offset. The cost is cubic.

hljs.highlight('[a](/'.repeat(n), { language: 'markdown' }), Node 26. Every point was taken in a fresh process, min of several runs, and only accepted when a fixed regex reference workload measured in that same process landed within 8% of its idle baseline:

input 11.12.0 this PR
2 KB 48 ms 23 ms
4 KB 379 ms 52 ms
8 KB 3.03 s 108 ms
16 KB 24.1 s 225 ms
32 KB 453 ms
128 KB 1.84 s

11.12.0 grows ×8 per doubling; with the bounds it grows ×2.

Bounding the label at 128 and the destination at 512 characters removes the cubic term. The remaining cost is proportional to the bound — holding the input at 16 KB and varying only the label bound gives 32 → 65 ms, 64 → 123 ms, 128 → 236 ms, 256 → 464 ms, 512 → 921 ms — so the numbers above can be traded for a tighter bound if you would rather have one.

Making the label deterministic instead (\[[^\]\n]+\]) would be faster still, but it changes which substrings count as links — a label containing ] stops matching — so I left that alone.

What it costs: on small adversarial inputs the bound is a net loss, because it forces up to 128 label attempts where the unbounded form gave up sooner. 500 B: 1.0 ms → 2.6 ms; 1 KB: 6.3 ms → 8.8 ms. On real markdown there is no cost — 200 README and CHANGELOG files from npm, 1.45 MB in total, take 51–54 ms before and 44–48 ms after.

Behaviour: I generated 9360 link forms — labels with brackets, nesting, escapes and unicode; every URL scheme the variants name; empty labels; reference links; the same forms wrapped in lists, quotes, headings and code spans; and truncated forms that never close — and diffed the rendered markup. Zero differences. As a check that the comparison could fail at all, perturbing the output classes makes it report 8022.

Tests: test/parser/markdown-link-backtracking.js, in the shape of the existing function-declaration-backtracking.js from #4362. With this commit reverted it fails at 26,057 ms against a 4,000 ms budget.

One thing worth flagging: test/regex's "should not cause polynomial backtracking" passes for markdown both before and after this change. It looks for a single repeated character that can reach from one unbounded quantifier to the next, and this attack pumps a five-character sequence ([a](/), so the check cannot see it. #4529 lists 17 more regexes found by other means; this rule is not among them — that issue flags markdown.js:25+26, which is the fenced-code rule.

Checklist

The five LINK variants each paired an unbounded lazy label with an unbounded
lazy destination, so text that opens a link and never closes one was retried
for the whole remaining input at every start offset: 8 KB of "[a](/" repeated
took 4.9s, and the cost grew by 8x per doubling.

Bounding both runs makes the cost linear in the input. Highlighting of real
markdown is unchanged: 200 README and CHANGELOG files (1.45 MB) take the same
time as before, and 9360 generated link forms render identically.

Assisted-by: Claude Opus 5 (high)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Build Size Report

Changes to minified artifacts in /build, after gzip compression.

5 files changed

Total change +88 B

View Changes
file base pr diff
es/core.min.js 8.16 KB 8.16 KB +1 B
es/highlight.min.js 8.16 KB 8.16 KB +1 B
es/languages/markdown.min.js 975 B 1.02 KB +42 B
highlight.min.js 8.2 KB 8.2 KB +1 B
languages/markdown.min.js 975 B 1.02 KB +43 B

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant