Always neutralize javascript: links, independent of fixRelativeURLs

README.md

@@ -151,8 +151,35 @@ Readability scans and scores HTML elements based on the number of words, links a
 
 ## Security
 
+Readability is a content **extractor**, not a sanitizer. The returned
+`Article::$content` (and `Article::$contentElement`) is HTML pulled from the
+source page — it is *not* safe to render as-is when the input is untrusted.
+
 If you're going to use Readability with untrusted input (whether in HTML or DOM form), we **strongly** recommend you use a sanitizer library like [HTML Purifier](https://github.com/ezyang/htmlpurifier) or [Symfony's HtmlSanitizer](https://symfony.com/doc/current/html_sanitizer.html) to avoid script injection when you use the output of Readability. We would also recommend using [CSP](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP) to add further defense-in-depth restrictions to what you allow the resulting content to do. The Firefox integration of reader mode uses both of these techniques itself. Sanitizing unsafe content out of the input is explicitly not something we aim to do as part of Readability itself - there are other good sanitizer libraries out there, use them!
 
+Readability removes `<script>`, `<style>` and `<noscript>` elements and
+neutralizes `<a href="javascript:...">` links (the latter now happens
+regardless of the `fixRelativeURLs` setting, as a defense-in-depth measure).
+This is **not** a substitute for a real sanitizer. In particular, the
+following can survive extraction and must be handled by your sanitizer:
+
+- Inline event-handler attributes (`onclick`, `onerror`, `onload`, …).
+- `data:` and other non-`http(s)` URIs on media — `src`/`srcset`/`poster` of
+  `img`/`source`/`video`/etc., including URLs promoted from lazy-loading
+  attributes such as `data-src`.
+- Embedded video players (`<iframe>`/`<embed>`/`<object>`) that are kept when
+  they match `allowedVideoRegex` — these are preserved with all their
+  attributes.
+
+Two operational notes for untrusted input:
+
+- Set `maxElemsToParse` (default `0`, meaning no limit) and cap the raw input
+  size yourself. The whole document is parsed into a DOM before that limit is
+  checked, so it bounds the extraction work rather than peak parse-time memory.
+- Treat `allowedVideoRegex` as trusted configuration. It is matched against
+  element markup from the (possibly attacker-controlled) document, so a
+  pathological pattern supplied here could cause catastrophic backtracking.
+
 ## Development and testing
 
 The test corpus is Mozilla's own `test-pages` set (plus a few PHP-specific pages), and content comparison uses a PHP port of Mozilla's structural DOM comparison, so Mozilla's expected files are used as-is.

UPGRADE.md

@@ -184,6 +184,7 @@ Debug messages are sent to the logger independently of the `debug` flag (which o
 - **The content is wrapped** in `<div id="readability-page-1" class="page">…</div>`, exactly as Readability.js outputs. If you post-process the HTML, account for the wrapper.
 - **The byline is always extracted, and inline bylines are removed from the content by default** (see `keepInlineByline` above). A 3.x install that never set `articleByline` kept the byline in the content.
 - **Relative URL fixing follows the WHATWG URL Standard** (what browsers and Readability.js do), via PHP 8.5's native `Uri\WhatWg\Url` or rowbot/url on PHP 8.4. Edge-case outputs may differ slightly from 3.x's RFC 3986 resolution (e.g. `https://example.com` serializes as `https://example.com/`).
+- **`javascript:` links are now neutralized regardless of `fixRelativeURLs`.** Previously this ran only when `fixRelativeURLs` was enabled; it now always runs (matching Readability.js), since stripping the scheme needs no base URL. Absolutizing relative URLs remains opt-in via `fixRelativeURLs`. This is defense-in-depth only — see the Security section of the README; you still need a real sanitizer for untrusted input.
 - **Encoding:** input is parsed with the encoding declared in the document, defaulting to UTF-8 (as a browser would). The 3.x mb_* guessing hacks are gone — supply UTF-8 or make sure the document declares its charset.
 - **Class attributes** are stripped by default as before (`keepClasses: false`), but the preserved-classes list now also honors `classesToPreserve`.
 

src/Readability.php

@@ -253,11 +253,12 @@ private function resolveBaseURI(): void
      */
     private function postProcessContent(\Dom\Element $articleContent): void
     {
-        // Readability cannot open relative uris so we convert them to absolute uris.
-        // (PHP: opt-in, since there is no live document to take a base URL from.)
-        if ($this->configuration->fixRelativeURLs && $this->baseURI !== null) {
-            $this->fixRelativeUris($articleContent);
-        }
+        // Neutralize javascript: links and (opt-in) convert relative uris to
+        // absolute ones. The javascript: handling always runs — it matches
+        // Readability.js, needs no base URL, and is a defense-in-depth measure.
+        // Absolutizing relative uris is opt-in (PHP has no live document to
+        // take a base URL from) and is gated inside fixRelativeUris().
+        $this->fixRelativeUris($articleContent);
 
         $this->simplifyNestedElements($articleContent);
 
@@ -360,6 +361,11 @@ private function isUrl(string $str): bool
      */
     private function fixRelativeUris(\Dom\Element $articleContent): void
     {
+        // Converting relative uris to absolute ones is opt-in and needs a base
+        // URI. Neutralizing javascript: links, however, always runs — it is a
+        // safety measure that needs no base URL.
+        $absolutize = $this->configuration->fixRelativeURLs && $this->baseURI !== null;
+
         $links = $this->getAllNodesWithTag($articleContent, ['a']);
         foreach ($links as $link) {
             $href = $link->getAttribute('href');
@@ -379,12 +385,16 @@ private function fixRelativeUris(\Dom\Element $articleContent): void
                         }
                         $link->parentNode->replaceChild($container, $link);
                     }
-                } else {
+                } elseif ($absolutize) {
                     $link->setAttribute('href', $this->toAbsoluteURI($href));
                 }
             }
         }
 
+        if (!$absolutize) {
+            return;
+        }
+
         $medias = $this->getAllNodesWithTag($articleContent, ['img', 'picture', 'figure', 'video', 'audio', 'source']);
         foreach ($medias as $media) {
             $src = $media->getAttribute('src');

test/ReadabilityUnitTest.php

@@ -140,6 +140,28 @@ public function testGetRowAndColumnCount(string $html, int $rows, int $columns):
         $this->assertSame(['rows' => $rows, 'columns' => $columns], self::invoke('getRowAndColumnCount', $table));
     }
 
+    /**
+     * javascript: links must be neutralized even with the default configuration
+     * (fixRelativeURLs off). Readability.js always strips them; decoupling this
+     * safety step from relative-URL absolutization keeps parity with upstream.
+     */
+    public function testJavascriptLinksNeutralizedWithDefaultConfig(): void
+    {
+        $filler = str_repeat('This is the body of the article with enough text to be selected. ', 12);
+        $html = '<html><body><article><p>'
+            . $filler
+            . '<a href="javascript:alert(1)">click me</a> '
+            . $filler
+            . '</p></article></body></html>';
+
+        // Default configuration: fixRelativeURLs is false.
+        $article = new Readability(new Configuration())->parse($html);
+
+        $this->assertStringNotContainsString('javascript:', $article->content);
+        // The link text is preserved (converted to a text node / span).
+        $this->assertStringContainsString('click me', $article->content);
+    }
+
     public static function getRowAndColumnCountProvider(): array
     {
         return [