From 05c1ceceac53555e7db5807ff99e1c97857178b0 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Wed, 8 Jul 2026 17:39:51 +0200 Subject: [PATCH] perf: linear nested [quote] and nested brackets (was O(n^2) DoS) Two converter/parser quadratic blowups found by a security audit: - BbcodeToDjot::parseQuotesWithDepth recursed on each closed quote's inner content and used unanchored preg_match per position, so deeply nested [quote] was O(n^2) (~19s for 90KB). Now a single stack-based pass, O(n). - The inline parser ran the char-by-char bracket-depth scan for every even when the text has no link/span trigger (, , ), so a balanced nested run like [[[[x]]]] was O(n^2) (~33s at 20k). parseLink now short-circuits on a memoized trigger-presence check (matching carve-php), O(n). Output unchanged; regression tests added. --- src/Converter/BbcodeToDjot.php | 88 +++++++++---------- src/Parser/InlineParser.php | 24 +++++ tests/TestCase/Converter/BbcodeToDjotTest.php | 13 +++ 3 files changed, 78 insertions(+), 47 deletions(-) diff --git a/src/Converter/BbcodeToDjot.php b/src/Converter/BbcodeToDjot.php index 9ea9d79..027907c 100644 --- a/src/Converter/BbcodeToDjot.php +++ b/src/Converter/BbcodeToDjot.php @@ -158,67 +158,61 @@ protected function convertQuotes(string $text): string */ protected function parseQuotesWithDepth(string $text): string { - $result = ''; $length = strlen($text); $i = 0; + // Single left-to-right pass with a stack of open-quote content buffers + // (O(n)). The previous version recursed on each closed quote's inner + // content and re-scanned it, which is O(n^2) on deeply nested + // `[quote]` (a converter DoS). Index 0 accumulates the top-level output; + // each `[quote]` pushes a level, each `[/quote]` pops one, formats it as + // a blockquote, and folds it into its parent -- producing the same + // output the recursion did for well-formed input. + /** @var array $contents */ + $contents = ['']; + /** @var array $authors */ + $authors = [null]; + $top = 0; while ($i < $length) { - // Check for opening quote tag - matches [quote], [quote=...], or [quote ...] - if (preg_match('/\[quote(?:[= ]([^\]]*))?\]/i', $text, $matches, 0, $i) && strpos($text, $matches[0], $i) === $i) { - $author = $matches[1] ?? null; - $tagLength = strlen($matches[0]); - $i += $tagLength; - - // Find the matching closing tag by tracking depth - $depth = 1; - $contentStart = $i; - - while ($i < $length) { - // Check for nested opening quote - if (preg_match('/\[quote(?:[= ][^\]]*)?\]/i', $text, $m, 0, $i) && strpos($text, $m[0], $i) === $i) { - $depth++; - $i += strlen($m[0]); - - continue; - } - - // Check for closing quote - if (preg_match('/\[\/quote\]/i', $text, $m, 0, $i) && strpos($text, $m[0], $i) === $i) { - $depth--; - if ($depth === 0) { - // Extract content and convert to Djot blockquote - $content = substr($text, $contentStart, $i - $contentStart); - $i += strlen($m[0]); - - // Recursively process nested quotes in content - $content = $this->parseQuotesWithDepth($content); - - // Convert to Djot blockquote format - $result .= $this->formatAsBlockquote($content, $author); - - continue 2; // Continue outer loop - } - $i += strlen($m[0]); + if (preg_match('/\G\[quote(?:[= ]([^\]]*))?\]/i', $text, $m, 0, $i)) { + $contents[] = ''; + $authors[] = $m[1] ?? null; + $top++; + $i += strlen($m[0]); - continue; - } + continue; + } - $i++; + if (preg_match('/\G\[\/quote\]/i', $text, $m, 0, $i)) { + $i += strlen($m[0]); + if ($top > 0) { + $blockquote = $this->formatAsBlockquote($contents[$top], $authors[$top]); + array_pop($contents); + array_pop($authors); + $top--; + $contents[$top] .= $blockquote; } - - // If we exit without finding closing tag, treat remaining as content - $content = substr($text, $contentStart); - $result .= $this->formatAsBlockquote($content, $author); + // A stray `[/quote]` with no open quote is dropped. continue; } - // Regular character, add to result - $result .= $text[$i]; + $contents[$top] .= $text[$i]; $i++; } - return $result; + // Unclosed quotes: format each remaining open level as a blockquote, + // innermost first, folding into its parent (matches the previous + // "content runs to end of input" behavior). + while ($top > 0) { + $blockquote = $this->formatAsBlockquote($contents[$top], $authors[$top]); + array_pop($contents); + array_pop($authors); + $top--; + $contents[$top] .= $blockquote; + } + + return $contents[0]; } /** diff --git a/src/Parser/InlineParser.php b/src/Parser/InlineParser.php index f1fce4f..6b8e37b 100644 --- a/src/Parser/InlineParser.php +++ b/src/Parser/InlineParser.php @@ -91,6 +91,15 @@ class InlineParser */ protected ?string $abbreviationPattern = null; + /** + * Memoized per-text check: does the text contain any link/span trigger + * (`](`, `][`, `]{`)? Lets parseLink skip the O(n) bracket-depth scan for + * trigger-free text, so a deeply nested `[[[[x]]]]` run stays linear. + */ + protected ?string $linkTriggerText = null; + + protected bool $linkTriggerPresent = false; + /** * Cached abbreviation keys for the current pattern * @@ -791,6 +800,21 @@ protected function parseLink(string $text, int $pos): ?array return null; } + // A link, reference, or inline span can only form when the matched `]` + // is directly followed by `(`, `[`, or `{`. If the text contains none of + // `](`, `][`, `]{`, nothing can start here, so skip the bracket-depth + // scan below -- otherwise a deeply nested run like `[[[[x]]]]` is + // O(n^2). The presence check is memoized per text. + if ($text !== $this->linkTriggerText) { + $this->linkTriggerText = $text; + $this->linkTriggerPresent = strpos($text, '](') !== false + || strpos($text, '][') !== false + || strpos($text, ']{') !== false; + } + if (!$this->linkTriggerPresent) { + return null; + } + // Find closing ] $bracketDepth = 1; $textEnd = $pos + 1; diff --git a/tests/TestCase/Converter/BbcodeToDjotTest.php b/tests/TestCase/Converter/BbcodeToDjotTest.php index d24a109..da216a6 100644 --- a/tests/TestCase/Converter/BbcodeToDjotTest.php +++ b/tests/TestCase/Converter/BbcodeToDjotTest.php @@ -392,4 +392,17 @@ public function testCodeLangCannotMintRawHtmlBlock(): void $this->assertStringNotContainsString('