diff --git a/.changelog/current/2898-local-download-warning.md b/.changelog/current/2898-local-download-warning.md new file mode 100644 index 000000000..13655bfc1 --- /dev/null +++ b/.changelog/current/2898-local-download-warning.md @@ -0,0 +1,3 @@ +# Fixed + +- Add a warning when accessing local servers diff --git a/cookbook.code-workspace b/cookbook.code-workspace index 00ad8ac0f..942f302a3 100644 --- a/cookbook.code-workspace +++ b/cookbook.code-workspace @@ -41,9 +41,10 @@ "**/vendor/**/{Tests,tests}/**", "**/.history/**", "**/vendor/**/vendor/**", - // "3rdparty/**" ".github/actions/**", - ".hook-checkout/**" + ".hook-checkout/**", + "${workspaceFolder:cookbook}/.github/actions/**", + "**/.github/actions/**" ], "cSpell.words": [ "Nextcloud" diff --git a/lib/Controller/Implementation/RecipeImplementation.php b/lib/Controller/Implementation/RecipeImplementation.php index b2e0416f3..9a7cddbd5 100644 --- a/lib/Controller/Implementation/RecipeImplementation.php +++ b/lib/Controller/Implementation/RecipeImplementation.php @@ -3,6 +3,7 @@ namespace OCA\Cookbook\Controller\Implementation; use Exception; +use OCA\Cookbook\Exception\InvalidDownloadURLException; use OCA\Cookbook\Exception\InvalidJSONFileException; use OCA\Cookbook\Exception\NoRecipeNameGivenException; use OCA\Cookbook\Exception\RecipeExistsException; @@ -56,6 +57,7 @@ public function __construct( AcceptHeaderParsingHelper $acceptHeaderParsingHelper, IL10N $iL10N, LoggerInterface $logger, + private string $userId, ) { $this->request = $request; $this->service = $recipeService; @@ -268,6 +270,44 @@ public function image($id) { } } + /** + * @todo Move this code into a dedicated class and service + */ + private function isDownloadUrlValid(string $url): bool { + $urlValid = filter_var($url, FILTER_VALIDATE_URL); + if ($urlValid === false) { + $this->logger->info('Download URL is not a valid URL: {url}', ['url' => $url]); + return false; + } + + $parsedUrl = parse_url($url); + + $scheme = $parsedUrl['scheme'] ?? ''; + $allowedSchemes = ['http', 'https']; + if (!in_array($scheme, $allowedSchemes, true)) { + $this->logger->info('Download URL uses invalid scheme: {scheme}', ['scheme' => $scheme]); + return false; + } + + $host = $parsedUrl['host'] ?? ''; + if (empty($host)) { + $this->logger->info('Download URL does not have a valid host: {url}', ['url' => $url]); + return false; + } + + if ($host === 'localhost') { + $this->logger->info('Download URL host is localhost, which is not allowed.'); + return false; + } + + if (filter_var($host, FILTER_VALIDATE_IP)) { + $this->logger->info('Download URL host is an IP address, which is not allowed: {host}', ['host' => $host]); + return false; + } + + return true; + } + /** * Trigger the import of a recipe. * @@ -282,6 +322,15 @@ public function import() { return new JSONResponse('Field "url" is required', 400); } + if (! $this->isDownloadUrlValid($data['url'])) { + $this->logger->warning('Attempt to download recipe by user {user} from invalid URL: {url}', ['url' => $data['url'], 'user' => $this->userId]); + // throw new InvalidDownloadURLException($this->l->t('The provided URL is not allowed.')); + $json = [ + 'msg' => $this->l->t('The provided URL is not allowed.'), + ]; + return new JSONResponse($json['msg'], 400); + } + try { $recipe_file = $this->service->downloadRecipe($data['url']); $recipe_json = $this->service->parseRecipeFile($recipe_file); diff --git a/lib/Exception/InvalidDownloadURLException.php b/lib/Exception/InvalidDownloadURLException.php new file mode 100644 index 000000000..e8392e3d3 --- /dev/null +++ b/lib/Exception/InvalidDownloadURLException.php @@ -0,0 +1,9 @@ +stubFilter, $this->acceptHeaderParser, $l, - $logger + $logger, + 'testuser', ); } @@ -753,4 +754,34 @@ public static function dataProviderIndex(): array { ], ]; } + + public static function dpInvalidImportURLs(): array { + return [ + 'emptyUrl' => [''], + 'nonHttp' => ['ftp://example.com/recipe.html'], + 'localUrl' => ['http://localhost/recipe.html'], + 'internalIp' => ['http://192.168.2.3/recipe.html'], + 'localhost' => ['http://127.0.0.1/recipe.html'], + 'localhostWithPort' => ['http://127.0.0.1:22/recipe.html'], + 'remoteIp' => ['http://8.8.8.8/recipe.html'], + 'localhost_ipv6' => ['http://::1/recipe.html'], + ]; + } + + /** + * @dataProvider dpInvalidImportURLs + * @param mixed $url + */ + public function testInvalidImports($url) { + $this->ensureCacheCheckTriggered(); + + $this->restParser->method('getParameters')->willReturn([ 'url' => $url ]); + + /** + * @var JSONResponse $ret + */ + $ret = $this->sut->import(); + + $this->assertEquals(400, $ret->getStatus()); + } }