security: harden URL schemes in non-HTML renderers (#255)

Follow-up to the always-on HTML hardening: the Markdown, plain-text and ANSI
renderers emitted link destinations and image sources verbatim, so a dangerous
scheme (javascript:/vbscript:/data:/file:) carried straight into a Markdown or
terminal export - markup that gets rendered or clicked elsewhere.

- Add Djot\Util\UrlSafety with the shared scheme denylist + sanitize helpers.
- MarkdownRenderer / AnsiRenderer blank a dangerous link/image URL; PlainText
  falls back to the link text. Safe and relative URLs are untouched.
- Refactor HtmlRenderer to delegate its baseline to UrlSafety (single source).

This is the renderer-side fix for the carry-through; the HtmlToDjot importer is
deliberately not a sanitizer (its output is re-rendered through these now-hardened
renderers).

Author: dereuromark

src/Renderer/AnsiRenderer.php

@@ -48,6 +48,7 @@
 use Djot\Node\Inline\Text;
 use Djot\Node\Node;
 use Djot\Renderer\Utility\AbbreviationBudgetTrait;
+use Djot\Util\UrlSafety;
 
 /**
  * Renders AST to ANSI-formatted terminal output
@@ -790,13 +791,13 @@ protected function renderCode(Code $node): string
     protected function renderLink(Link $node): string
     {
         $text = $this->renderChildren($node);
-        $url = $node->getDestination();
+        $url = UrlSafety::sanitize($node->getDestination() ?? '');
 
         // OSC 8 hyperlink support (for terminals that support it)
         // Format: \033]8;;URL\033\\TEXT\033]8;;\033\\
         $styled = $this->style($text, self::UNDERLINE . self::FG_BLUE);
 
-        if ($url !== null && $url !== '' && $url !== $text) {
+        if ($url !== '' && $url !== $text) {
             $styled .= $this->style(' (' . $url . ')', self::DIM);
         }
 

src/Renderer/HtmlRenderer.php

@@ -52,6 +52,7 @@
 use Djot\Renderer\Utility\EventDispatcherTrait;
 use Djot\SafeMode;
 use Djot\Util\StringUtil;
+use Djot\Util\UrlSafety;
 
 /**
  * Renders AST to HTML
@@ -1111,14 +1112,6 @@ protected function getRenderableAttributes(Node $node, array $exclude = []): arr
         return $attrs;
     }
 
-    /**
-     * URL schemes that are always neutralized in attribute values and in
-     * `href` / `src`, regardless of safe mode.
-     *
-     * @var array<string>
-     */
-    private const DANGEROUS_VALUE_SCHEMES = ['javascript', 'vbscript', 'data', 'file'];
-
     /**
      * Always-on attribute hardening, applied regardless of safe mode.
      *
@@ -1152,12 +1145,8 @@ protected function sanitizeAttributes(array $attrs): array
      */
     protected function sanitizeAttributeValue(string $name, string $value): string
     {
-        $colon = strpos($value, ':');
-        if ($colon !== false) {
-            $scheme = strtolower((string)preg_replace('/[\x00-\x20]+/', '', substr($value, 0, $colon)));
-            if (in_array($scheme, self::DANGEROUS_VALUE_SCHEMES, true)) {
-                return '';
-            }
+        if (UrlSafety::hasDangerousScheme($value)) {
+            return '';
         }
         if ($name === 'style' && $this->hasDangerousCss($value)) {
             return '';
@@ -1213,14 +1202,7 @@ static function (array $m): string {
      */
     protected function sanitizeUrlBaseline(string $url): string
     {
-        $probe = (string)preg_replace('/[\x00-\x20]+/', '', $url);
-        if (preg_match('/^([a-zA-Z][a-zA-Z0-9+.\-]*):/', $probe, $m) === 1) {
-            if (in_array(strtolower($m[1]), self::DANGEROUS_VALUE_SCHEMES, true)) {
-                return '';
-            }
-        }
-
-        return $url;
+        return UrlSafety::sanitize($url);
     }
 
     /**

src/Renderer/MarkdownRenderer.php

@@ -50,6 +50,7 @@
 use Djot\Renderer\Utility\AbbreviationBudgetTrait;
 use Djot\Renderer\Utility\EventDispatcherTrait;
 use Djot\Util\StringUtil;
+use Djot\Util\UrlSafety;
 
 /**
  * Renders AST to Markdown (CommonMark compatible where possible)
@@ -411,7 +412,7 @@ protected function renderCode(Code $node): string
     protected function renderLink(Link $node): string
     {
         $text = $this->renderChildren($node);
-        $url = $node->getDestination();
+        $url = UrlSafety::sanitize($node->getDestination() ?? '');
         $title = $node->getTitle();
 
         if ($title !== null) {
@@ -424,7 +425,7 @@ protected function renderLink(Link $node): string
     protected function renderImage(Image $node): string
     {
         $alt = $node->getAlt();
-        $src = $node->getSource();
+        $src = UrlSafety::sanitize($node->getSource());
         $title = $node->getTitle();
 
         if ($title !== null) {

src/Renderer/PlainTextRenderer.php

@@ -38,6 +38,7 @@
 use Djot\Node\Inline\Text;
 use Djot\Node\Node;
 use Djot\Renderer\Utility\EventDispatcherTrait;
+use Djot\Util\UrlSafety;
 
 /**
  * Renders AST to plain text
@@ -294,7 +295,12 @@ protected function renderFootnote(Footnote $node): string
 
     protected function renderLink(Link $node): string
     {
-        return $node->getDestination() ?? $this->renderChildren($node);
+        $url = $node->getDestination();
+        if ($url === null || UrlSafety::hasDangerousScheme($url)) {
+            return $this->renderChildren($node);
+        }
+
+        return $url;
     }
 
     /**

src/Util/UrlSafety.php

@@ -0,0 +1,45 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Djot\Util;
+
+/**
+ * Always-on URL scheme hardening shared by every renderer.
+ *
+ * A link destination or image source whose (normalized) scheme is on the
+ * dangerous denylist (`javascript`, `vbscript`, `data`, `file`) is neutralized
+ * regardless of safe mode or output format - there is no legitimate reason to
+ * emit such a URL from untrusted markup, and a non-HTML export (Markdown, plain
+ * text) carries it to wherever it is rendered next. Scheme detection strips C0
+ * controls + spaces first to defeat `java\tscript:` style evasion.
+ */
+final class UrlSafety
+{
+    /**
+     * @var array<string>
+     */
+    public const DANGEROUS_SCHEMES = ['javascript', 'vbscript', 'data', 'file'];
+
+    /**
+     * True when the URL carries a dangerous denylist scheme. Scheme-less and
+     * relative URLs, and every non-denylist scheme, return false.
+     */
+    public static function hasDangerousScheme(string $url): bool
+    {
+        $probe = (string)preg_replace('/[\x00-\x20]+/', '', $url);
+        if (preg_match('/^([a-zA-Z][a-zA-Z0-9+.\-]*):/', $probe, $m) === 1) {
+            return in_array(strtolower($m[1]), self::DANGEROUS_SCHEMES, true);
+        }
+
+        return false;
+    }
+
+    /**
+     * Blank a URL that carries a dangerous scheme; otherwise return it unchanged.
+     */
+    public static function sanitize(string $url): string
+    {
+        return self::hasDangerousScheme($url) ? '' : $url;
+    }
+}

tests/TestCase/Renderer/NonHtmlUrlHardeningTest.php

@@ -0,0 +1,79 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Djot\Test\TestCase\Renderer;
+
+use Djot\DjotConverter;
+use Djot\Renderer\AnsiRenderer;
+use Djot\Renderer\MarkdownRenderer;
+use Djot\Renderer\PlainTextRenderer;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Non-HTML renderers must not carry a dangerous URL scheme into their output.
+ *
+ * The HTML renderer blanks `javascript:` / `vbscript:` / `data:` / `file:` URLs,
+ * but a Markdown / plain-text / ANSI export of the same document is markup that
+ * gets rendered (or clicked) somewhere else - so the same denylist applies to
+ * every output format, not just HTML.
+ */
+class NonHtmlUrlHardeningTest extends TestCase
+{
+    /**
+     * @return array<string, array{0: string}>
+     */
+    public static function dangerousUrlProvider(): array
+    {
+        return [
+            'javascript' => ['javascript:alert(1)'],
+            'vbscript' => ['vbscript:msgbox(1)'],
+            'data' => ['data:text/html,<script>alert(1)</script>'],
+            'file' => ['file:///etc/passwd'],
+            'evasion' => ["java\tscript:alert(1)"],
+        ];
+    }
+
+    #[DataProvider('dangerousUrlProvider')]
+    public function testMarkdownBlanksDangerousLinkUrl(string $url): void
+    {
+        $converter = DjotConverter::create(renderer: new MarkdownRenderer());
+        $result = $converter->convert('[x](' . $url . ')');
+
+        $this->assertStringNotContainsString('script:', $result);
+        $this->assertStringNotContainsString('data:', $result);
+        $this->assertStringNotContainsString('file:', $result);
+        $this->assertStringContainsString('[x]()', $result);
+    }
+
+    public function testMarkdownBlanksDangerousImageUrl(): void
+    {
+        $converter = DjotConverter::create(renderer: new MarkdownRenderer());
+        $this->assertStringContainsString('![a]()', $converter->convert('![a](data:text/html,x)'));
+    }
+
+    public function testMarkdownPreservesSafeUrl(): void
+    {
+        $converter = DjotConverter::create(renderer: new MarkdownRenderer());
+        $this->assertStringContainsString('[x](https://ok.example)', $converter->convert('[x](https://ok.example)'));
+    }
+
+    public function testPlainTextDropsDangerousUrlToLinkText(): void
+    {
+        $converter = DjotConverter::create(renderer: new PlainTextRenderer());
+        $result = $converter->convert('[label](javascript:alert(1))');
+
+        $this->assertStringNotContainsString('javascript', $result);
+        $this->assertStringContainsString('label', $result);
+    }
+
+    public function testAnsiDropsDangerousUrl(): void
+    {
+        $converter = DjotConverter::create(renderer: new AnsiRenderer());
+        $plain = preg_replace('/\033\[[0-9;]*m/', '', $converter->convert('[label](vbscript:x)')) ?? '';
+
+        $this->assertStringNotContainsString('vbscript', $plain);
+        $this->assertStringContainsString('label', $plain);
+    }
+}