Browse Source

Sanitizer: block data: URI scheme while keeping data:image for images (#9417)

* Sanitizer: block data: URI scheme, keep data:image for images

FreshRSS_SimplePieCustom only disallowed the `javascript` scheme, so
`data:text/html,...` URIs in feed content survived
sanitization on <a href>, <iframe src>, <audio|video|source|track src>.
The default CSP neutralises these on FreshRSS pages, but clients and
deployments without that CSP (API WebViews, extensions using amendCsp(),
proxies stripping headers) could execute them. Same class as
GHSA-3fw6-j8m4-82vj / #9272.

- Sanitize::is_allowed_uri(): element/attribute-aware check that still
  permits inert `data:image/...` on img[src], image[src], video[poster]
  while blocking `data:` elsewhere; replace_urls() now uses it.
  is_allowed_scheme() is unchanged for backward compatibility.
- SimplePieCustom: disallow_uri_schemes(['javascript','data']).
- Tests: 4 blocked-input cases, a data:image keep-alive test and an
  `unsafe:` prefix test. They fail without the patch and pass with it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Sync SimplePie
https://github.com/FreshRSS/simplepie/pull/92

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
ImperfectBlu 3 days ago
parent
commit
0187b15c8e

+ 2 - 1
app/Models/SimplePieCustom.php

@@ -64,7 +64,8 @@ final class FreshRSS_SimplePieCustom extends \SimplePie\SimplePie
 		$this->rename_attributes(['id', 'class']);
 		$this->allow_aria_attr(true);
 		$this->allow_data_attr(true);
-		$this->disallow_uri_schemes(['javascript']);
+		// `data:` is still allowed for images (see Sanitize::is_allowed_uri()), but blocked on navigational/framing attributes
+		$this->disallow_uri_schemes(['javascript', 'data']);
 		$this->allowed_html_attributes([
 			// HTML
 			'dir',

+ 1 - 1
lib/composer.json

@@ -18,7 +18,7 @@
 		"marienfressinaud/lib_opml": "dev-main#f0e850b6394af90b898daf0e65fcc7363457b844",
 		"phpgt/cssxpath": "v1.6.0",
 		"phpmailer/phpmailer": "7.1.1",
-		"simplepie/simplepie": "dev-freshrss#caa6737811d75301720d07b30d9b53a4ab0b969d"
+		"simplepie/simplepie": "dev-freshrss#fcfbf7769eb9b3725a1f07b609771b0d7a12b0c7"
 	},
 	"config": {
 		"sort-packages": true,

+ 31 - 1
lib/simplepie/simplepie/src/Sanitize.php

@@ -677,7 +677,7 @@ class Sanitize implements RegistryAware
                         $value = $this->registry->call(Misc::class, 'absolutize_url', [$element->getAttribute($attribute), $this->base]);
                         if ($value !== false) {
                             // Block disallowed URI protocols (e.g. javascript:), otherwise force HTTPS where applicable
-                            $value = ($this->disallowed_uri_schemes !== [] && !$this->is_allowed_scheme($value))
+                            $value = ($this->disallowed_uri_schemes !== [] && !$this->is_allowed_uri($tag, $attribute, $value))
                                 ? 'unsafe:' . $value
                                 : $this->https_url($value);
                             $element->setAttribute($attribute, $value);
@@ -779,6 +779,36 @@ class Sanitize implements RegistryAware
         return !in_array($scheme, $this->disallowed_uri_schemes, true);
     }
 
+    /**
+     * Elements/attributes on which a `data:` URI carrying an image is still allowed
+     * even when the `data` scheme is otherwise disallowed. Such URIs are inert
+     * (an `<img>` never executes script), whereas `data:` on navigational or framing
+     * attributes (`<a href>`, `<iframe src>`, ...) may load attacker-controlled HTML.
+     * @var array<string, list<string>>
+     */
+    private const DATA_IMAGE_URI_ALLOWED = [
+        'image' => ['src'],
+        'img' => ['src'],
+        'video' => ['poster'],
+    ];
+
+    /**
+     * Like {@see is_allowed_scheme()}, but aware of the element and attribute the URI sits on,
+     * so that `data:image/...` can stay allowed on image-bearing attributes while `data:` is
+     * blocked elsewhere (e.g. `<a href="data:text/html,...">`, `<iframe src="data:...">`).
+     */
+    public function is_allowed_uri(string $tag, string $attribute, string $uri): bool
+    {
+        $pos = strpos($uri, ':');
+        if ($pos !== false && strtolower(substr($uri, 0, $pos)) === 'data'
+            && in_array('data', $this->disallowed_uri_schemes, true)
+            && in_array($attribute, self::DATA_IMAGE_URI_ALLOWED[strtolower($tag)] ?? [], true)
+            && preg_match('%^data:image/[a-z0-9.+-]+[;,]%i', $uri) === 1) {
+            return true;
+        }
+        return $this->is_allowed_scheme($uri);
+    }
+
     /**
      * Block disallowed URI schemes (protocols) on elements carrying an href attribute.
      *

+ 19 - 0
tests/app/Models/SimplePieCustomTest.php

@@ -36,6 +36,10 @@ final class SimplePieCustomTest extends \PHPUnit\Framework\TestCase {
 		yield 'inline event handler' => ['<img src="x" onerror="alert(1)">', 'onerror'];
 		yield 'JavaScript URL' => ['<a href="javascript:alert(1)">click</a>', 'href="javascript:'];
 		yield 'style tag' => ['<style>body{display:none}</style>Hello', '<style'];
+		yield 'data:text/html URL in link' => ['<a href="data:text/html,<script>alert(1)</script>">click</a>', 'href="data:'];
+		yield 'data:text/html URL in iframe' => ['<iframe src="data:text/html,<script>alert(1)</script>"></iframe>', 'src="data:'];
+		yield 'data:text/html URL in image' => ['<img src="data:text/html,<script>alert(1)</script>">', 'src="data:'];
+		yield 'data:image URL in link' => ['<a href="data:image/png;base64,iVBORw0KGgo=">click</a>', 'href="data:'];
 	}
 
 	public static function test_sanitizeHTML_whenSafeHtml_keepsAllowedTags(): void {
@@ -92,4 +96,19 @@ final class SimplePieCustomTest extends \PHPUnit\Framework\TestCase {
 		yield 'double unclosed tag' => ['<b> <b>x', 10, 'x'];
 		yield 'triple unclosed tag' => [' <b><b><b>y', 20, 'y'];
 	}
+
+	/**
+	 * Inline `data:image/...` URIs are inert in `<img>` and widely used by feeds, so they must survive
+	 * while `data:` is blocked on navigational/framing attributes (see provideMaliciousHtml()).
+	 */
+	public static function test_sanitizeHTML_whenDataImageUri_keepsItOnImageElements(): void {
+		$png = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=';
+		self::assertSame('<img src="' . $png . '" alt="">', FreshRSS_SimplePieCustom::sanitizeHTML('<img src="' . $png . '" alt="">'));
+		self::assertStringContainsString('poster="' . $png . '"', FreshRSS_SimplePieCustom::sanitizeHTML('<video poster="' . $png . '"></video>'));
+	}
+
+	public static function test_sanitizeHTML_whenDisallowedScheme_prefixesUnsafe(): void {
+		$result = FreshRSS_SimplePieCustom::sanitizeHTML('<a href="data:text/html,x">click</a>');
+		self::assertStringContainsString('href="unsafe:data:text/html,x"', $result);
+	}
 }