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

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
pull/54019/head
Ferdinand Thiessen 2025-07-07 19:11:28 +07:00
parent a1f4b59997
commit 074b994218
No known key found for this signature in database
GPG Key ID: 45FAE7268762B400
1 changed files with 48 additions and 3 deletions

@ -23,7 +23,6 @@ use OCP\AppFramework\Http;
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;
@ -45,7 +44,9 @@ use OCP\IL10N;
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 OCP\Util;
use Psr\Log\LoggerInterface;
@ -129,9 +130,8 @@ class AppSettingsController extends Controller {
* @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, '"');
@ -161,6 +161,26 @@ class AppSettingsController extends Controller {
$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);
@ -182,6 +202,31 @@ class AppSettingsController extends Controller {
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