diff --git a/.composer-require-checker.config.json b/.composer-require-checker.config.json index 6761b781c10..1acf299d78f 100644 --- a/.composer-require-checker.config.json +++ b/.composer-require-checker.config.json @@ -34,7 +34,7 @@ "GLPI_MARKETPLACE_PRERELEASES", "GLPI_NETWORK_REGISTRATION_API_URL", "GLPI_NETWORK_MAIL", "GLPI_NETWORK_SERVICES", "GLPI_PICTURE_DIR", "GLPI_PLUGIN_DOC_DIR", "GLPI_RSS_DIR", "GLPI_SESSION_DIR", "GLPI_TELEMETRY_URI", "GLPI_TMP_DIR", "GLPI_UPLOAD_DIR", "GLPI_USE_CSRF_CHECK", "GLPI_USE_IDOR_CHECK", - "GLPI_USER_AGENT_EXTRA_COMMENTS", "GLPI_VAR_DIR", "GLPI_CENTRAL_WARNINGS", + "GLPI_USER_AGENT_EXTRA_COMMENTS", "GLPI_VAR_DIR", "GLPI_CENTRAL_WARNINGS", "GLPI_SERVERSIDE_URL_ALLOWLIST", "// GLPI optionnal constants", "GLPI_FORCE_MAIL", "GLPI_LOG_LVL", diff --git a/css/legacy/includes/_planning.scss b/css/legacy/includes/_planning.scss index c92659f059a..18c8a5f73ee 100644 --- a/css/legacy/includes/_planning.scss +++ b/css/legacy/includes/_planning.scss @@ -108,6 +108,11 @@ text-overflow: ellipsis; display: inline-block; white-space: nowrap; + + > i { + color: $warning; + float: right; + } } .filter-icon { diff --git a/inc/based_config.php b/inc/based_config.php index ab52f7ec79a..0664d086c13 100644 --- a/inc/based_config.php +++ b/inc/based_config.php @@ -82,6 +82,11 @@ 'GLPI_USE_IDOR_CHECK' => '1', 'GLPI_IDOR_EXPIRES' => '7200', 'GLPI_ALLOW_IFRAME_IN_RICH_TEXT' => false, + 'GLPI_SERVERSIDE_URL_ALLOWLIST' => [ + // allowlist (regex format) of URL that can be fetched from server side (used for RSS feeds and external calendars, among others) + // URL will be considered as safe as long as it matches at least one entry of the allowlist + '/^(https?|feed):\/\/[^@:]+(\/.*)?$/', // only accept http/https/feed protocols, and reject presence of @ (username) and : (protocol) in host part of URL + ], // Constants related to GLPI Project / GLPI Network external services 'GLPI_TELEMETRY_URI' => 'https://telemetry.glpi-project.org', // Telemetry project URL @@ -127,7 +132,7 @@ // This logic is quiet simple and is not made to handle chain inheritance. $inherit_pattern = '/\{(?GLPI_[\w]+)\}/'; foreach ($constants as $key => $value) { - if (!defined($key) && !preg_match($inherit_pattern, $value)) { + if (!defined($key) && (!is_string($value) || !preg_match($inherit_pattern, $value))) { define($key, $value); } } diff --git a/src/Planning.php b/src/Planning.php index 8962207d7b9..fde20e9ccbe 100644 --- a/src/Planning.php +++ b/src/Planning.php @@ -1003,7 +1003,13 @@ class='" . $filter_data['type'] . $expanded . "'>"; echo ""; } - echo ""; + echo ""; $color = self::$palette_bg[$params['filter_color_index']]; if (isset($filter_data['color']) && !empty($filter_data['color'])) { @@ -1396,6 +1402,15 @@ public static function showAddExternalForm() */ public static function sendAddExternalForm($params = []) { + if (!Toolbox::isUrlSafe($params['url'])) { + Session::addMessageAfterRedirect( + sprintf(__('URL "%s" is not allowed by your administrator.'), $params['url']), + false, + ERROR + ); + return; + } + $_SESSION['glpi_plannings']['plannings']['external_' . md5($params['url'])] = [ 'color' => self::getPaletteColor('bg', $_SESSION['glpi_plannings_color_index']), 'display' => true, diff --git a/src/RSSFeed.php b/src/RSSFeed.php index 2c597d598e6..06ee7d7eda0 100644 --- a/src/RSSFeed.php +++ b/src/RSSFeed.php @@ -603,12 +603,11 @@ public static function displayTabContentForItem(CommonGLPI $item, $tabnum = 1, $ return false; } - - /** - * @see CommonDBTM::prepareInputForAdd() - **/ public function prepareInputForAdd($input) { + if (!$this->checkUrlInput($input['url'])) { + return false; + } if ($feed = self::getRSSFeed($input['url'])) { $input['have_error'] = 0; @@ -628,12 +627,11 @@ public function prepareInputForAdd($input) return $input; } - - /** - * @see CommonDBTM::prepareInputForAdd() - **/ public function prepareInputForUpdate($input) { + if (array_key_exists('url', $input) && !$this->checkUrlInput($input['url'])) { + return false; + } if ( empty($input['name']) @@ -648,6 +646,24 @@ public function prepareInputForUpdate($input) return $input; } + /** + * Check URL given in input. + * @param string $url + * @return bool + */ + private function checkUrlInput(string $url): bool + { + if (parse_url($url) === false) { + Session::addMessageAfterRedirect(__('Feed URL is invalid.'), false, ERROR); + return false; + } elseif (!Toolbox::isUrlSafe($url)) { + Session::addMessageAfterRedirect(sprintf(__('URL "%s" is not allowed by your administrator.'), $url), false, ERROR); + return false; + } + + return true; + } + public function pre_updateInDB() { @@ -769,7 +785,11 @@ public function showForm($ID, array $options = []) echo ""; echo "" . __('Error retrieving RSS feed') . ""; echo ""; - echo Dropdown::getYesNo($this->fields['have_error']); + if ($this->fields['have_error'] && !Toolbox::isUrlSafe($this->fields['url'])) { + echo sprintf(__('URL "%s" is not allowed by your administrator.'), $this->fields['url']); + } else { + echo Dropdown::getYesNo($this->fields['have_error']); + } echo ""; if ($this->fields['have_error']) { echo "" . __('RSS feeds found'); @@ -823,15 +843,11 @@ public function showFeedContent() if (!$this->canViewItem()) { return false; } - $feed = self::getRSSFeed($this->fields['url'], $this->fields['refresh_rate']); $rss_feed = [ 'items' => [] ]; echo "
"; - if (!$feed || $feed->error()) { - $rss_feed['error'] = !$feed ? __('Error retrieving RSS feed') : $feed->error(); - $this->setError(true); - } else { + if ($feed = self::getRSSFeed($this->fields['url'], $this->fields['refresh_rate'])) { $this->setError(false); $rss_feed['title'] = $feed->get_title(); foreach ($feed->get_items(0, $this->fields['max_items']) as $item) { @@ -842,6 +858,11 @@ public function showFeedContent() 'content' => $item->get_content() ]; } + } else { + $rss_feed['error'] = !Toolbox::isUrlSafe($this->fields['url']) + ? sprintf(__('URL "%s" is not allowed by your administrator.'), $this->fields['url']) + : __('Error retrieving RSS feed'); + $this->setError(true); } TemplateRenderer::getInstance()->display('components/rss_feed.html.twig', [ @@ -857,6 +878,9 @@ public function showFeedContent() **/ public function showDiscoveredFeeds() { + if (!Toolbox::isUrlSafe($this->fields['url'])) { + return; + } $feed = new SimplePie(); $feed->set_cache_location(GLPI_RSS_DIR); @@ -903,6 +927,10 @@ public static function getRSSFeed($url, $cache_duration = DAY_TIMESTAMP) { global $CFG_GLPI; + if (!Toolbox::isUrlSafe($url)) { + return false; + } + if (Sanitizer::isHtmlEncoded($url)) { $url = Sanitizer::decodeHtmlSpecialChars($url); } diff --git a/src/System/Status/StatusChecker.php b/src/System/Status/StatusChecker.php index b0341f75e0e..ef152df73c3 100644 --- a/src/System/Status/StatusChecker.php +++ b/src/System/Status/StatusChecker.php @@ -392,11 +392,21 @@ public static function getCASStatus($public_only = true): array $url .= ':' . (int)$CFG_GLPI['cas_port']; } $url .= '/' . $CFG_GLPI['cas_uri']; - $data = Toolbox::getURLContent($url); - if (!empty($data)) { - $status['status'] = self::STATUS_OK; + if (Toolbox::isUrlSafe($url)) { + $data = Toolbox::getURLContent($url); + if (!empty($data)) { + $status['status'] = self::STATUS_OK; + } else { + $status['status'] = self::STATUS_PROBLEM; + } } else { - $status['status'] = self::STATUS_PROBLEM; + $status['status'] = self::STATUS_NO_DATA; + if (!$public_only) { + $status['status_msg'] = sprintf( + __('URL "%s" is not considered safe and cannot be fetched from GLPI server.'), + $url + ); + } } } } diff --git a/src/Toolbox.php b/src/Toolbox.php index f3e8778044c..cac9734e106 100644 --- a/src/Toolbox.php +++ b/src/Toolbox.php @@ -1094,6 +1094,10 @@ public static function checkNewVersionAvailable() //parse github releases (get last version number) $error = ""; $json_gh_releases = self::getURLContent("https://api.github.com/repos/glpi-project/glpi/releases", $error); + if (empty($json_gh_releases)) { + return $error; + } + $all_gh_releases = json_decode($json_gh_releases, true); $released_tags = []; foreach ($all_gh_releases as $release) { @@ -1329,6 +1333,35 @@ public static function getTimestampTimeUnits($time) } + /** + * Check an url is safe. + * Used to mitigate SSRF exploits. + * + * @since 10.0.3 + * + * @param string $url URL to check + * @param array $allowlist Allowlist (regex array) + * + * @return bool + */ + public static function isUrlSafe(string $url, array $allowlist = GLPI_SERVERSIDE_URL_ALLOWLIST): bool + { + foreach ($allowlist as $allow_regex) { + $result = preg_match($allow_regex, $url); + if ($result === false) { + trigger_error( + sprintf('Unable to validate URL safeness. Following regex is probably invalid: "%s".', $allow_regex), + E_USER_WARNING + ); + } elseif ($result === 1) { + return true; + } + } + + return false; + } + + /** * Get a web page. Use proxy if configured * @@ -1340,7 +1373,8 @@ public static function getTimestampTimeUnits($time) **/ public static function getURLContent($url, &$msgerr = null, $rec = 0) { - $content = self::callCurl($url); + $curl_error = null; + $content = self::callCurl($url, [], $msgerr, $curl_error, true); return $content; } @@ -1354,10 +1388,24 @@ public static function getURLContent($url, &$msgerr = null, $rec = 0) * * @return string */ - public static function callCurl($url, array $eopts = [], &$msgerr = null, &$curl_error = null) - { + public static function callCurl( + $url, + array $eopts = [], + &$msgerr = null, + &$curl_error = null, + bool $check_url_safeness = false + ) { global $CFG_GLPI; + if ($check_url_safeness && !Toolbox::isUrlSafe($url)) { + $msgerr = sprintf( + __('URL "%s" is not considered safe and cannot be fetched from GLPI server.'), + $url + ); + trigger_error(sprintf('Unsafe URL "%s" fetching has been blocked.', $url), E_USER_NOTICE); + return ''; + } + $content = ""; $taburl = parse_url($url); diff --git a/templates/components/rss_feed.html.twig b/templates/components/rss_feed.html.twig index 297c5ff6461..6d4ecbf6139 100644 --- a/templates/components/rss_feed.html.twig +++ b/templates/components/rss_feed.html.twig @@ -33,7 +33,7 @@
{% if rss_feed.error is defined and rss_feed.error is not null %} - {{ __('Error retrieving RSS feed') }} + {{ rss_feed.error }} {% endif %} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 8a64b8781b1..2eba1739d05 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -55,6 +55,14 @@ ] ); +define( + 'GLPI_SERVERSIDE_URL_ALLOWLIST', + [ + '/^(https?|feed):\/\/[^@:]+(\/.*)?$/', // default allowlist entry + '/^file:\/\/.*\.ics$/', // calendar mockups + ] +); + define('TU_USER', '_test_user'); define('TU_PASS', 'PhpUnit_4'); diff --git a/tests/functionnal/Toolbox.php b/tests/functionnal/Toolbox.php index ecbda6622a6..001739dac97 100644 --- a/tests/functionnal/Toolbox.php +++ b/tests/functionnal/Toolbox.php @@ -1431,4 +1431,89 @@ public function testGetMioSizeFromString(string $size, $expected): void $result = \Toolbox::getMioSizeFromString($size); $this->variable($result)->isEqualTo($expected); } + + protected function safeUrlProvider(): iterable + { + // Invalid URLs are refused + yield [ + 'url' => '', + 'expected' => false, + ]; + yield [ + 'url' => ' ', + 'expected' => false, + ]; + + // Invalid schemes are refused + yield [ + 'url' => 'file://tmp/test', + 'expected' => false, + ]; + yield [ + 'url' => 'test://localhost/', + 'expected' => false, + ]; + + // Local file are refused + yield [ + 'url' => '//tmp/test', + 'expected' => false, + ]; + + // http, https and feed URLs are accepted, unless they contains a user or port information + foreach (['http', 'https', 'feed'] as $scheme) { + foreach (['', '/', '/path/to/feed.php'] as $path) { + yield [ + 'url' => sprintf('%s://localhost%s', $scheme, $path), + 'expected' => true, + ]; + yield [ + 'url' => sprintf('%s://localhost:8080%s', $scheme, $path), + 'expected' => false, + ]; + yield [ + 'url' => sprintf('%s://test@localhost%s', $scheme, $path), + 'expected' => false, + ]; + yield [ + 'url' => sprintf('%s://test:pass@localhost%s', $scheme, $path), + 'expected' => false, + ]; + } + } + + // Custom allowlist with multiple entries + $custom_allowlist = [ + '|^https://\w+:[^/]+@calendar.mydomain.tld/|', + '|//intra.mydomain.tld/|', + ]; + yield [ + 'url' => 'https://calendar.external.tld/', + 'expected' => false, + 'allowlist' => $custom_allowlist, + ]; + yield [ + 'url' => 'https://user:pass@calendar.mydomain.tld/', + 'expected' => true, // validates first item of allowlist + 'allowlist' => $custom_allowlist, + ]; + yield [ + 'url' => 'http://intra.mydomain.tld/news.feed.php', + 'expected' => true, // validates second item of allowlist + 'allowlist' => $custom_allowlist, + ]; + } + + + /** + * @dataProvider safeUrlProvider + */ + public function testIsUrlSafe(string $url, bool $expected, ?array $allowlist = null): void + { + $params = [$url]; + if ($allowlist !== null) { + $params[] = $allowlist; + } + $this->boolean(call_user_func_array('Toolbox::isUrlSafe', $params))->isEqualTo($expected); + } }