Browse Source

Disallow fetching of non remote uri (#9215)

* Disallow redirection to non-remote URLs

Fix https://github.com/FreshRSS/FreshRSS/security/advisories/GHSA-fgq3-88jp-7rj9

* Disable unnecessary stream wrappers

* Add `is_remote_uri()` check at beginning of `httpGet()`

* SimplePie syntax

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

---------

Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
Inverle 2 weeks ago
parent
commit
c7e6ab76fe

+ 8 - 0
app/Models/SimplePieFetch.php

@@ -51,6 +51,14 @@ final class FreshRSS_SimplePieFetch extends \SimplePie\File
 
 	#[\Override]
 	protected function on_http_response($response, array $curl_options = []): void {
+		if (!\SimplePie\Misc::is_remote_uri($this->get_final_requested_uri())) {
+			$this->set_status_code(0);
+			$this->set_body_content('');
+			$this->error = 'Fetching non-remote URLs is not permitted: “' . $this->get_final_requested_uri() . '“';
+			$this->success = false;
+			$response = '';
+		}
+
 		if (FreshRSS_Context::systemConf()->simplepie_syslog_enabled) {
 			syslog(LOG_INFO, 'FreshRSS SimplePie GET ' . $this->get_status_code() . ' ' . \SimplePie\Misc::url_remove_credentials($this->get_final_requested_uri()));
 		}

+ 8 - 2
app/Utils/httpUtil.php

@@ -464,6 +464,11 @@ final class FreshRSS_http_Util {
 	 *   * `-500` `curl_init()` failure.
 	 */
 	public static function httpGet(string $url, ?string $cachePath = null, string $type = 'html', array $attributes = [], array $curl_options = []): array {
+		if (!\SimplePie\Misc::is_remote_uri($url)) {
+			Minz_Log::warning('Error fetching content: malformed URL “' . $url . '“');
+			return ['body' => '', 'effective_url' => '', 'redirect_count' => 0, 'fail' => true, 'status' => -500, 'error' => ''];
+		}
+
 		$limits = FreshRSS_Context::systemConf()->limits;
 		$feed_timeout = empty($attributes['timeout']) || !is_numeric($attributes['timeout']) ? 0 : intval($attributes['timeout']);
 
@@ -658,8 +663,9 @@ final class FreshRSS_http_Util {
 			if (in_array($c_status, [301, 302, 303, 307, 308], true)) {
 				// Handle the redirect by making another request
 				$location = \SimplePie\Misc::absolutize_url($headers['location'] ?? $url, $url);
-				if ($location === false) {
-					$location = $url;
+				if ($location === false || !\SimplePie\Misc::is_remote_uri($location)) {
+					Minz_Log::warning('Invalid redirect location: malformed URL “' . ($headers['location'] ?? $url) . '“');
+					break;
 				}
 				if (!self::compareURLOrigins($url, $location)) {
 					unset($curl_options[CURLOPT_COOKIE]);

+ 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#37ebf581e60ce90ebee7d79181de43cd8f63f936"
+		"simplepie/simplepie": "dev-freshrss#0b0908c4c05462a781e029119c8bf3f98e9b8b80"
 	},
 	"config": {
 		"sort-packages": true,

+ 19 - 0
lib/lib_rss.php

@@ -47,6 +47,25 @@ function actualize_mutex_file(string $tmpPath, string $dataPath): string {
 	return $tmpPath . '/actualize.' . hash('sha256', realpath($dataPath) ?: $dataPath) . '.freshrss.lock';
 }
 
+/**
+ * Disable stream wrappers that the application does not need.
+ *
+ * For temporary usage of a previously unregistered wrapper, `stream_wrapper_restore()` can be used.
+ */
+function unregister_unsafe_protocols(): void {
+	$registered_wrappers = stream_get_wrappers();
+	$allowed_wrappers = ['file', 'php'];
+	foreach ($registered_wrappers as $protocol) {
+		if (!in_array($protocol, $allowed_wrappers, true)) {
+			stream_wrapper_unregister($protocol);
+		}
+	}
+}
+
+if (!defined('WITH_COMPOSER')) {
+	unregister_unsafe_protocols();
+}
+
 //<Auto-loading>
 function classAutoloader(string $class): void {
 	if (str_starts_with($class, 'FreshRSS')) {

+ 17 - 3
lib/simplepie/simplepie/src/File.php

@@ -104,7 +104,7 @@ class File implements Response
             $this->permanent_url = $url;
         }
         $this->useragent = $useragent;
-        if (preg_match('/^http(s)?:\/\//i', $url)) {
+        if (\SimplePie\Misc::is_remote_uri($url)) {
             if ($useragent === null) {
                 $useragent = (string) ini_get('user_agent');
                 $this->useragent = $useragent;
@@ -235,7 +235,8 @@ class File implements Response
                                 ($locationHeader = $this->get_header_line('location')) !== '' && ($this->redirects < $redirects || $redirects === -1)) { // FreshRSS: added infinite redirects for -1
                                 $this->redirects++;
                                 $location = \SimplePie\Misc::absolutize_url($locationHeader, $url);
-                                if ($location === false) {
+                                if ($location === false || !\SimplePie\Misc::is_remote_uri($location)) {
+                                    $this->status_code = 0;
                                     $this->error = "Invalid redirect location, trying to base “{$locationHeader}” onto “{$url}”";
                                     $this->success = false;
                                     return;
@@ -259,6 +260,7 @@ class File implements Response
                                     throw new \InvalidArgumentException('Malformed URL: ' . $url);
                                 }
                                 if (($url_parts_to = parse_url(strtolower($location))) === false) {
+                                    $this->status_code = 0;
                                     $this->error = "Invalid redirect location: malformed URL “{$location}”";
                                     $this->success = false;
                                     return;
@@ -369,7 +371,8 @@ class File implements Response
                                 $this->redirects++;
                                 $location = \SimplePie\Misc::absolutize_url($locationHeader, $url);
                                 $this->permanentUrlMutable = $this->permanentUrlMutable && ($this->status_code == 301 || $this->status_code == 308);
-                                if ($location === false) {
+                                if ($location === false || !\SimplePie\Misc::is_remote_uri($location)) {
+                                    $this->status_code = 0;
                                     $this->error = "Invalid redirect location, trying to base “{$locationHeader}” onto “{$url}”";
                                     $this->success = false;
                                     return;
@@ -381,6 +384,7 @@ class File implements Response
                                     throw new \InvalidArgumentException('Malformed URL: ' . $url);
                                 }
                                 if (($url_parts_to = parse_url(strtolower($location))) === false) {
+                                    $this->status_code = 0;
                                     $this->error = "Invalid redirect location: malformed URL “{$location}”";
                                     $this->success = false;
                                     return;
@@ -522,6 +526,11 @@ class File implements Response
         return (int) $this->status_code;
     }
 
+    public function set_status_code(int $status_code): void
+    {
+        $this->status_code = $status_code;
+    }
+
     public function get_headers(): array
     {
         $this->maybe_update_headers();
@@ -564,6 +573,11 @@ class File implements Response
         return (string) $this->body;
     }
 
+    public function set_body_content(string $body): void
+    {
+        $this->body = $body;
+    }
+
     /**
      * Check if the $headers property was changed and update the internal state accordingly.
      */