Merge pull request #54029 from nextcloud/backport/54019/stable30

[stable30] fix(settings): verify source of app-discover media

Author: susnux

apps/settings/lib/Controller/AppSettingsController.php

@@ -23,7 +23,6 @@
 use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\Attribute\OpenAPI;
 use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
-use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\Http\ContentSecurityPolicy;
 use OCP\AppFramework\Http\FileDisplayResponse;
 use OCP\AppFramework\Http\JSONResponse;
@@ -43,7 +42,9 @@
 use OCP\INavigationManager;
 use OCP\IRequest;
 use OCP\IURLGenerator;
+use OCP\IUserSession;
 use OCP\L10N\IFactory;
+use OCP\Security\RateLimiting\ILimiter;
 use OCP\Server;
 use Psr\Log\LoggerInterface;
 
@@ -126,9 +127,8 @@ public function getAppDiscoverJSON(): JSONResponse {
 	 * @param string $image
 	 * @throws \Exception
 	 */
-	#[PublicPage]
 	#[NoCSRFRequired]
-	public function getAppDiscoverMedia(string $fileName): Response {
+	public function getAppDiscoverMedia(string $fileName, ILimiter $limiter, IUserSession $session): Response {
 		$getEtag = $this->discoverFetcher->getETag() ?? date('Y-m');
 		$etag = trim($getEtag, '"');
 
@@ -158,6 +158,26 @@ public function getAppDiscoverMedia(string $fileName): Response {
 		$file = reset($file);
 		// If not found request from Web
 		if ($file === false) {
+			$user = $session->getUser();
+			// this route is not public thus we can assume a user is logged-in
+			assert($user !== null);
+			// Register a user request to throttle fetching external data
+			// this will prevent using the server for DoS of other systems.
+			$limiter->registerUserRequest(
+				'settings-discover-media',
+				// allow up to 24 media requests per hour
+				// this should be a sane default when a completely new section is loaded
+				// keep in mind browsers request all files from a source-set
+				24,
+				60 * 60,
+				$user,
+			);
+
+			if (!$this->checkCanDownloadMedia($fileName)) {
+				$this->logger->warning('Tried to load media files for app discover section from untrusted source');
+				return new NotFoundResponse(Http::STATUS_BAD_REQUEST);
+			}
+
 			try {
 				$client = $this->clientService->newClient();
 				$fileResponse = $client->get($fileName);
@@ -179,6 +199,31 @@ public function getAppDiscoverMedia(string $fileName): Response {
 		return $response;
 	}
 
+	private function checkCanDownloadMedia(string $filename): bool {
+		$urlInfo = parse_url($filename);
+		if (!isset($urlInfo['host']) || !isset($urlInfo['path'])) {
+			return false;
+		}
+
+		// Always allowed hosts
+		if ($urlInfo['host'] === 'nextcloud.com') {
+			return true;
+		}
+
+		// Hosts that need further verification
+		// Github is only allowed if from our organization
+		$ALLOWED_HOSTS = ['github.com', 'raw.githubusercontent.com'];
+		if (!in_array($urlInfo['host'], $ALLOWED_HOSTS)) {
+			return false;
+		}
+
+		if (str_starts_with($urlInfo['path'], '/nextcloud/') || str_starts_with($urlInfo['path'], '/nextcloud-gmbh/')) {
+			return true;
+		}
+
+		return false;
+	}
+
 	/**
 	 * Remove orphaned folders from the image cache that do not match the current etag
 	 * @param ISimpleFolder $folder The folder to clear