Explorar o código

SimplePie: Disallow `javascript:` URI protocol (#8263)

Follow-up of https://github.com/FreshRSS/FreshRSS/pull/7924
https://github.com/FreshRSS/simplepie/pull/80
https://github.com/FreshRSS/simplepie/pull/65

* SimplePie: Disallow `javascript:` URI protocol

* Sync SimplePie

* Update code to work with SimplePie again

* Partial revert previous commit

* Bump SimplePie

---------

Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
Inverle hai 2 semanas
pai
achega
1349c7c1a9

+ 1 - 0
app/Models/SimplePieCustom.php

@@ -64,6 +64,7 @@ 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']);
 		$this->allowed_html_attributes([
 			// HTML
 			'dir',

+ 1 - 1
lib/composer.json

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

+ 5 - 2
lib/simplepie/simplepie/src/Item.php

@@ -883,10 +883,13 @@ class Item implements RegistryAware
                 $this->data['links'][$key] = array_unique($this->data['links'][$key]);
             }
 
-            // Apply HTTPS policy to all links
+            // Apply sanitization and HTTPS policy to all links
+            $sanitize = $this->get_sanitize();
             foreach ($this->data['links'] as &$links) {
                 foreach ($links as &$link) {
-                    $link = $this->get_sanitize()->https_url($link);
+                    $link = ($sanitize->disallowed_uri_schemes !== [] && !$sanitize->is_allowed_scheme($link))
+                                ? 'unsafe:' . $link
+                                : $sanitize->https_url($link);
                 }
             }
         }

+ 71 - 4
lib/simplepie/simplepie/src/Sanitize.php

@@ -58,6 +58,8 @@ class Sanitize implements RegistryAware
     public $allow_data_attr = true;
     /** @var bool */
     public $allow_aria_attr = true;
+    /** @var string[] */
+    public $disallowed_uri_schemes = ['javascript'];
     /** @var array<string, array<string, string>> */
     public $add_attributes = ['audio' => ['preload' => 'none'], 'iframe' => ['sandbox' => 'allow-scripts allow-same-origin'], 'video' => ['preload' => 'none']];
     /** @var bool */
@@ -280,6 +282,14 @@ class Sanitize implements RegistryAware
         $this->allow_aria_attr = $allow;
     }
 
+    /**
+     * @param string[] $schemes List of URI schemes (protocols) to disallow
+     */
+    public function disallow_uri_schemes(array $schemes = ['javascript']): void
+    {
+        $this->disallowed_uri_schemes = $schemes;
+    }
+
     /**
      * @return void
      */
@@ -352,8 +362,8 @@ class Sanitize implements RegistryAware
      * containing URLs that need to be resolved relative to the feed
      *
      * Defaults to |a|@href, |area|@href, |audio|@src, |blockquote|@cite,
-     * |del|@cite, |form|@action, |img|@longdesc, |img|@src, |input|@src,
-     * |ins|@cite, |q|@cite, |source|@src, |video|@src
+     * |del|@cite, |form|@action, |iframe|@src, |img|@longdesc, |img|@src,
+     * |input|@src, |ins|@cite, |q|@cite, |source|@src, |video|@src
      *
      * @since 1.0
      * @param array<string, string|string[]>|null $element_attribute Element/attribute key/value pairs, null for default
@@ -369,6 +379,7 @@ class Sanitize implements RegistryAware
                 'blockquote' => 'cite',
                 'del' => 'cite',
                 'form' => 'action',
+                'iframe' => 'src',
                 'img' => [
                     'longdesc',
                     'src'
@@ -535,12 +546,20 @@ class Sanitize implements RegistryAware
                     }
                 }
 
-                // Replace relative URLs
+                // Replace relative URLs and blocks disallowed URI schemes (protocols)
                 $this->base = $base;
                 foreach ($this->replace_url_attributes as $element => $attributes) {
                     $this->replace_urls($document, $element, $attributes);
                 }
 
+                // MathML and SVG allow href on arbitrary descendants,
+                // so require a walk of the DOM
+                if ($this->disallowed_uri_schemes !== []
+                    && ($document->getElementsByTagName('math')->length > 0
+                        || $document->getElementsByTagName('svg')->length > 0)) {
+                    $this->block_disallowed_schemes_in_descendants($xpath);
+                }
+
                 // If image handling (caching, etc.) is enabled, cache and rewrite all the image tags.
                 if ($this->image_handler !== '' && $this->enable_cache) {
                     $images = $document->getElementsByTagName('img');
@@ -657,7 +676,10 @@ class Sanitize implements RegistryAware
                     if ($element->hasAttribute($attribute)) {
                         $value = $this->registry->call(Misc::class, 'absolutize_url', [$element->getAttribute($attribute), $this->base]);
                         if ($value !== false) {
-                            $value = $this->https_url($value);
+                            // Block disallowed URI protocols (e.g. javascript:), otherwise force HTTPS where applicable
+                            $value = ($this->disallowed_uri_schemes !== [] && !$this->is_allowed_scheme($value))
+                                ? 'unsafe:' . $value
+                                : $this->https_url($value);
                             $element->setAttribute($attribute, $value);
                         }
                     }
@@ -742,6 +764,51 @@ class Sanitize implements RegistryAware
         }
     }
 
+    public function is_allowed_scheme(string $uri): bool
+    {
+        $pos = strpos($uri, ':');
+        if ($pos === false) {
+            return true;
+        }
+        $scheme = strtolower(substr($uri, 0, $pos));
+        if (!ctype_alnum($scheme)) {
+            return false;
+        }
+        return !in_array($scheme, $this->disallowed_uri_schemes, true);
+    }
+
+    /**
+     * Block disallowed URI schemes (protocols) on elements carrying an href attribute.
+     *
+     * This handles MathML and SVG elements which permit href on arbitrary descendant elements
+     * For SVG, also checks xlink:href attribute.
+     */
+    private function block_disallowed_schemes_in_descendants(\DOMXPath $xpath): void
+    {
+        // Note: content is parsed via loadHTML(), which does not namespace-process attributes
+        $elements = $xpath->query('.//*[self::math or self::svg]/descendant-or-self::*[@href or @*[name()="xlink:href"]]');
+        if ($elements === false) {
+            return;
+        }
+        foreach ($elements as $element) {
+            if (!($element instanceof \DOMElement)) {
+                continue;
+            }
+            if ($element->hasAttribute('href')) {
+                $href = $element->getAttribute('href');
+                if (!$this->is_allowed_scheme($href)) {
+                    $element->setAttribute('href', 'unsafe:' . $href);
+                }
+            }
+            if ($element->hasAttribute('xlink:href')) {
+                $href = $element->getAttribute('xlink:href');
+                if (!$this->is_allowed_scheme($href)) {
+                    $element->setAttribute('xlink:href', 'unsafe:' . $href);
+                }
+            }
+        }
+    }
+
     /**
      * @param int-mask-of<SimplePie::CONSTRUCT_*> $type
      * @return void

+ 15 - 0
lib/simplepie/simplepie/src/SimplePie.php

@@ -692,6 +692,13 @@ class SimplePie
      */
     public $allow_aria_attr = true;
 
+    /**
+     * @var string[] Stores array of disallowed URI schemes (protocols)
+     * @see SimplePie::disallow_uri_schemes()
+     * @access private
+     */
+    public $disallowed_uri_schemes = ['javascript'];
+
     /**
      * @var bool Should we throw exceptions, or use the old-style error property?
      * @access private
@@ -1589,6 +1596,14 @@ class SimplePie
         $this->sanitize->allow_aria_attr($allow);
     }
 
+    /**
+     * @param string[] $schemes List of schemes (protocols) to disallow
+     */
+    public function disallow_uri_schemes(array $schemes = ['javascript']): void
+    {
+        $this->sanitize->disallow_uri_schemes($schemes);
+    }
+
     /**
      * @return void
      */