security: ignore round-trip raw attributes on untrusted HTML import; bound abbreviation expansion (#254)

Ports two confirmed hardening fixes from the downstream carve-php fork, on top
of the prior always-on attribute/URL hardening baseline.

HtmlToDjot round-trip injection:
- The reverse converter re-emitted the data-djot-src (block) and data-djot-raw
  (inline) round-trip attributes verbatim as raw Djot, so untrusted HTML could
  smuggle a raw-HTML block and inject live markup (e.g. a script element) into
  the converted output. Both attributes are now ignored by default; honoring
  them is opt-in via new HtmlToDjot(trustedRoundTrip: true) and propagated
  through the recursive inline sub-conversion. The internal Tabs round-trip
  generators, which consume djot's own rendered HTML, use the trusted converter
  so faithful round-trips are preserved.

Abbreviation output-amplification DoS:
- Each abbreviation occurrence re-emitted its full definition, so a tiny source
  with a huge definition and many occurrences expanded to definition_len times
  occurrence_count bytes (a RAM-exhaustion DoS). A per-render byte budget,
  max(1MB, 8 * sourceLength), now bounds cumulative expansion across the HTML,
  Markdown and ANSI renderers; once the budget is exceeded an occurrence
  degrades to plain key text. Document carries the source length, set in
  DjotConverter::parse().

Adds HtmlToDjotRoundTripSecurityTest and AbbreviationBudgetTest; existing
round-trip-fidelity tests use the trusted converter explicitly.

Author: dereuromark

src/Converter/HtmlToDjot.php

@@ -23,9 +23,30 @@
  * Key Djot requirements handled:
  * - Blank lines required around block elements (headings, code blocks, lists)
  * - Nested lists require blank line before the nested portion
+ *
+ * SECURITY: this converter is NOT a sanitizer. Its output is Djot markup that
+ * may still contain content derived from the input; render untrusted input with
+ * safe mode enabled on the downstream renderer. By default the converter IGNORES
+ * the `data-djot-src` / `data-djot-raw` round-trip attributes on the input (they
+ * would otherwise be re-emitted verbatim as raw Djot, letting a crafted attribute
+ * smuggle a raw-HTML block -> live <script> into the output). Only enable
+ * round-trip extraction via the constructor flag when the HTML is TRUSTED (e.g.
+ * produced by djot itself for a faithful round-trip).
  */
 class HtmlToDjot
 {
+    /**
+     * When true, trust and re-emit the `data-djot-src` / `data-djot-raw`
+     * round-trip attributes on the input. Default false: untrusted HTML must not
+     * be able to smuggle raw Djot (incl. a raw-HTML block) through them.
+     */
+    protected bool $trustedRoundTrip = false;
+
+    public function __construct(bool $trustedRoundTrip = false)
+    {
+        $this->trustedRoundTrip = $trustedRoundTrip;
+    }
+
     protected int $listDepth = 0;
 
     protected bool $inPre = false;
@@ -948,6 +969,14 @@ protected function processPreBlock(DOMElement $node): string
 
     protected function extractRoundTripSource(DOMElement $node, string $tagName): ?string
     {
+        // Untrusted HTML must not be able to smuggle raw Djot through a
+        // `data-djot-src` attribute (it is emitted verbatim, so a crafted value
+        // could inject a raw-HTML block -> live <script>). Only honor it when the
+        // caller explicitly trusts the source.
+        if (!$this->trustedRoundTrip) {
+            return null;
+        }
+
         if (!$node->hasAttribute('data-djot-src')) {
             return null;
         }
@@ -1852,8 +1881,12 @@ protected function processSpan(DOMElement $node): string
             return '\\' . $node->textContent;
         }
 
-        // Check for raw inline content (round-trip support)
-        if ($node->hasAttribute('data-djot-raw')) {
+        // Check for raw inline content (round-trip support). Only honor it for
+        // trusted input: `data-djot-raw` is re-emitted verbatim as a raw inline
+        // span (`` `...`{=html} ``), so a crafted attribute on untrusted HTML
+        // could smuggle live markup. Untrusted input falls through to ordinary
+        // span processing (getElementAttributes drops the data-djot-* attribute).
+        if ($this->trustedRoundTrip && $node->hasAttribute('data-djot-raw')) {
             return $this->processRawInline($node);
         }
 
@@ -2224,7 +2257,10 @@ protected function formatCaptionText(string $captionText): string
 
     protected function convertInlineFragmentToDjot(string $html): string
     {
-        $converter = new self();
+        // Propagate the trust setting so a trusted round-trip parent keeps
+        // honoring inner round-trip attributes in the recursive sub-conversion
+        // (and an untrusted parent keeps ignoring them).
+        $converter = new self($this->trustedRoundTrip);
         $converter->preserveTextWhitespace = true;
 
         $doc = new DOMDocument();

src/DjotConverter.php

@@ -417,6 +417,10 @@ public function parse(string $djot): Document
 
         $document = $this->parser->parse($djot);
 
+        // Record the source size so render-time DoS budgets (abbreviation
+        // expansion) can scale with the input rather than a fixed cap.
+        $document->setSourceLength(strlen($djot));
+
         foreach ($this->extensions as $extension) {
             if ($extension instanceof ParsedDocumentExtensionInterface) {
                 $extension->afterParse($document);

src/Extension/TabsExtension.php

@@ -582,7 +582,7 @@ protected function reconstructChildToDjot(Node $child, HtmlRenderer $renderer):
             $child instanceof DefinitionList => $this->renderDefinitionListToDjot($child, $renderer),
             $child instanceof Div => $this->renderDivToDjot($child, $renderer),
             $child instanceof Table => $this->renderTableToDjot($child, $renderer),
-            default => rtrim((new HtmlToDjot())->convert($renderer->renderNodeFragment($child)), "\n"),
+            default => rtrim((new HtmlToDjot(trustedRoundTrip: true))->convert($renderer->renderNodeFragment($child)), "\n"),
         };
     }
 
@@ -592,9 +592,9 @@ protected function renderDefinitionListToDjot(DefinitionList $list, HtmlRenderer
 
         foreach ($list->getChildren() as $child) {
             if ($child instanceof DefinitionTerm) {
-                $lines[] = rtrim((new HtmlToDjot())->convert($renderer->renderNodeFragment($child)), "\n");
+                $lines[] = rtrim((new HtmlToDjot(trustedRoundTrip: true))->convert($renderer->renderNodeFragment($child)), "\n");
             } elseif ($child instanceof DefinitionDescription) {
-                $content = rtrim((new HtmlToDjot())->convert($renderer->renderNodeFragment($child)), "\n");
+                $content = rtrim((new HtmlToDjot(trustedRoundTrip: true))->convert($renderer->renderNodeFragment($child)), "\n");
                 $lines[] = ': ' . $content;
             }
         }
@@ -655,7 +655,7 @@ protected function renderTableToDjot(Table $table, HtmlRenderer $renderer): stri
                     continue;
                 }
 
-                $cells[] = rtrim((new HtmlToDjot())->convert($renderer->renderNodeFragment($cell)), "\n");
+                $cells[] = rtrim((new HtmlToDjot(trustedRoundTrip: true))->convert($renderer->renderNodeFragment($cell)), "\n");
                 if ($row->isHeader() || !isset($alignments[$cellIndex])) {
                     $alignments[$cellIndex] = $cell->getAlignment();
                 }

src/Node/Document.php

@@ -16,11 +16,35 @@ class Document extends Node
      */
     protected array $abbreviations = [];
 
+    /**
+     * Byte length of the original source the document was parsed from.
+     *
+     * Used to scale render-time DoS budgets (e.g. abbreviation expansion).
+     * Defaults to 0 for documents built directly rather than parsed.
+     */
+    protected int $sourceLength = 0;
+
     public function getType(): string
     {
         return 'document';
     }
 
+    /**
+     * Get the byte length of the original parsed source.
+     */
+    public function getSourceLength(): int
+    {
+        return $this->sourceLength;
+    }
+
+    /**
+     * Set the byte length of the original parsed source.
+     */
+    public function setSourceLength(int $sourceLength): void
+    {
+        $this->sourceLength = $sourceLength;
+    }
+
     /**
      * Get abbreviation definitions
      *

src/Renderer/AnsiRenderer.php

@@ -47,6 +47,7 @@
 use Djot\Node\Inline\Symbol;
 use Djot\Node\Inline\Text;
 use Djot\Node\Node;
+use Djot\Renderer\Utility\AbbreviationBudgetTrait;
 
 /**
  * Renders AST to ANSI-formatted terminal output
@@ -56,6 +57,8 @@
  */
 class AnsiRenderer implements RendererInterface
 {
+    use AbbreviationBudgetTrait;
+
     // ANSI escape codes
     /**
      * @var string
@@ -367,6 +370,8 @@ public function getSoftBreakMode(): SoftBreakMode
 
     public function render(Document $document): string
     {
+        $this->resetAbbreviationBudget($document->getSourceLength());
+
         $output = $this->renderChildren($document);
 
         // Normalize multiple blank lines
@@ -921,6 +926,13 @@ protected function renderCaption(Caption $node): string
     protected function renderAbbreviation(Abbreviation $node): string
     {
         $text = $this->renderChildren($node);
+
+        // DoS guard: once the cumulative expansion bytes would exceed the
+        // budget, degrade to plain key text (no parenthesized definition).
+        if (!$this->chargeAbbreviationExpansion($node->getTitle())) {
+            return $text;
+        }
+
         $title = $node->getTitle();
 
         // Show abbreviation with definition in parentheses

src/Renderer/HtmlRenderer.php

@@ -48,6 +48,7 @@
 use Djot\Node\Inline\Symbol;
 use Djot\Node\Inline\Text;
 use Djot\Node\Node;
+use Djot\Renderer\Utility\AbbreviationBudgetTrait;
 use Djot\Renderer\Utility\EventDispatcherTrait;
 use Djot\SafeMode;
 use Djot\Util\StringUtil;
@@ -57,6 +58,7 @@
  */
 class HtmlRenderer implements RendererInterface
 {
+    use AbbreviationBudgetTrait;
     use EventDispatcherTrait;
 
     protected SoftBreakMode $softBreakMode = SoftBreakMode::Newline;
@@ -304,6 +306,7 @@ public function render(Document $document): string
             $this->sharedRenderContext,
             function () use ($document): string {
                 $this->sharedRenderContext->reset();
+                $this->resetAbbreviationBudget($document->getSourceLength());
 
                 $html = $this->renderDocumentWithSections($document);
 
@@ -1060,9 +1063,16 @@ protected function renderDelete(Delete $node): string
 
     protected function renderAbbreviation(Abbreviation $node): string
     {
-        $attrs = $this->renderAttributes($node);
         $title = $node->getTitle();
 
+        // DoS guard: once the cumulative expansion bytes would exceed the
+        // budget, degrade to plain key text (no <abbr> wrapper, no title).
+        if (!$this->chargeAbbreviationExpansion($title)) {
+            return $this->renderChildren($node);
+        }
+
+        $attrs = $this->renderAttributes($node);
+
         return '<abbr title="' . $this->escapeAttribute($title) . '"' . $attrs . '>'
             . $this->renderChildren($node) . '</abbr>';
     }

src/Renderer/MarkdownRenderer.php

@@ -47,6 +47,7 @@
 use Djot\Node\Inline\Symbol;
 use Djot\Node\Inline\Text;
 use Djot\Node\Node;
+use Djot\Renderer\Utility\AbbreviationBudgetTrait;
 use Djot\Renderer\Utility\EventDispatcherTrait;
 use Djot\Util\StringUtil;
 
@@ -66,6 +67,7 @@
  */
 class MarkdownRenderer implements RendererInterface
 {
+    use AbbreviationBudgetTrait;
     use EventDispatcherTrait;
 
     protected int $listDepth = 0;
@@ -96,6 +98,8 @@ public function getSoftBreakMode(): SoftBreakMode
 
     public function render(Document $document): string
     {
+        $this->resetAbbreviationBudget($document->getSourceLength());
+
         $markdown = $this->renderChildren($document);
 
         // Normalize multiple blank lines
@@ -528,12 +532,21 @@ protected function renderCaption(Caption $node): string
      */
     protected function renderAbbreviation(Abbreviation $node): string
     {
+        // DoS guard: once the cumulative expansion bytes would exceed the
+        // budget, degrade to plain key text (no <abbr> wrapper, no title).
+        // Return the children as ordinary Markdown text - NOT the HTML-escaped
+        // form below, which is only correct inside the raw <abbr> element (a
+        // bare `&`/`<` in the key must stay literal in Markdown output).
+        if (!$this->chargeAbbreviationExpansion($node->getTitle())) {
+            return $this->renderChildren($node);
+        }
+
         // The whole element is raw inline HTML, so both the title (attribute)
         // and the text (element content) need HTML escaping, NOT Markdown text
         // escaping: a `"` in the title or a `<` in the text would otherwise
         // break the tag / be misparsed as markup downstream.
-        $title = htmlspecialchars($node->getTitle(), ENT_QUOTES, 'UTF-8');
         $text = htmlspecialchars($this->renderChildren($node), ENT_QUOTES, 'UTF-8');
+        $title = htmlspecialchars($node->getTitle(), ENT_QUOTES, 'UTF-8');
 
         return '<abbr title="' . $title . '">' . $text . '</abbr>';
     }

src/Renderer/Utility/AbbreviationBudgetTrait.php

@@ -0,0 +1,84 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Djot\Renderer\Utility;
+
+/**
+ * Bounds the cumulative bytes contributed by abbreviation expansion across a
+ * single render, guarding against an output-amplification (memory) DoS.
+ *
+ * Each occurrence of an abbreviation re-emits its full definition (the `title`
+ * of an `<abbr>` element, the `(definition)` suffix in ANSI, etc.). A tiny
+ * source such as `*[HT]: <50KB of text>` followed by many `HT` occurrences
+ * would otherwise expand to `definition_len * occurrence_count` bytes
+ * (hundreds of MB), which PHP happily allocates - a true RAM-exhaustion DoS.
+ *
+ * Policy:
+ *   budget = max(BUDGET_BASE, BUDGET_FACTOR * sourceByteLength)
+ * Once the next occurrence's expansion would exceed the budget, that occurrence
+ * (and every subsequent one) degrades gracefully to its plain key text only -
+ * no `<abbr>` wrapper, no title. The budget sits far above any real document,
+ * so normal output is byte-identical.
+ *
+ * The counter is reset per full render call (resetAbbreviationBudget()).
+ */
+trait AbbreviationBudgetTrait
+{
+    /**
+     * Base (floor) budget in bytes, applied even for tiny sources.
+     *
+     * @var int
+     */
+    protected const ABBREVIATION_BUDGET_BASE = 1000000;
+
+    /**
+     * Multiplier applied to the source byte length.
+     *
+     * @var int
+     */
+    protected const ABBREVIATION_BUDGET_FACTOR = 8;
+
+    /**
+     * Cumulative expansion bytes already emitted in the current render.
+     */
+    protected int $abbreviationExpansionBytes = 0;
+
+    /**
+     * Computed budget for the current render (max of base and factor*source).
+     */
+    protected int $abbreviationBudget = self::ABBREVIATION_BUDGET_BASE;
+
+    /**
+     * Reset the budget counter and (re)compute the budget for a fresh render.
+     */
+    protected function resetAbbreviationBudget(int $sourceLength): void
+    {
+        $this->abbreviationExpansionBytes = 0;
+        $this->abbreviationBudget = max(
+            self::ABBREVIATION_BUDGET_BASE,
+            self::ABBREVIATION_BUDGET_FACTOR * $sourceLength,
+        );
+    }
+
+    /**
+     * Charge a single abbreviation occurrence against the budget.
+     *
+     * @param string $expansion The definition text whose bytes are emitted.
+     *
+     * @return bool True if the expansion fits within budget and may be emitted
+     *   (the bytes are charged); false if it would exceed the budget and the
+     *   occurrence must degrade to plain key text.
+     */
+    protected function chargeAbbreviationExpansion(string $expansion): bool
+    {
+        $cost = strlen($expansion);
+        if ($this->abbreviationExpansionBytes + $cost > $this->abbreviationBudget) {
+            return false;
+        }
+
+        $this->abbreviationExpansionBytes += $cost;
+
+        return true;
+    }
+}