Add warning on local downloads

.changelog/current/2898-local-download-warning.md

@@ -0,0 +1,3 @@
+# Fixed
+
+- Add a warning when accessing local servers

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"

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 @@
 		AcceptHeaderParsingHelper $acceptHeaderParsingHelper,
 		IL10N $iL10N,
 		LoggerInterface $logger,
+		private string $userId,
 	) {
 		$this->request = $request;
 		$this->service = $recipeService;
@@ -119,7 +121,7 @@
 	 *
 	 * @param $id The id of the recipe in question
 	 * @return JSONResponse
 	 * @todo Parameter id is never used. Fix that
 	 */
 	public function update($id) {
 		$this->dbCacheService->triggerCheck();
@@ -268,6 +270,44 @@
 		}
 	}
 
+	/**
+	 * @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 @@
 			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);

lib/Exception/InvalidDownloadURLException.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace OCA\Cookbook\Exception;
+
+class InvalidDownloadURLException extends \Exception {
+	public function __construct($message = '', $code = 0, $previous = null) {
+		parent::__construct($message, $code, $previous);
+	}
+}

tests/Unit/Controller/Implementation/RecipeImplementationTest.php

@@ -81,7 +81,8 @@
 			$this->stubFilter,
 			$this->acceptHeaderParser,
 			$l,
-			$logger
+			$logger,
+			'testuser',
 		);
 	}
 
@@ -335,7 +336,7 @@
 
 	/**
 	 * @dataProvider dpSearch
 	 * @todo no implementation in controller
 	 * @param mixed $query
 	 * @param mixed $recipes
 	 */
@@ -590,8 +591,8 @@
 
 	/**
 	 * @dataProvider dataProviderImage
 	 * @todo Assert on image data/file name
 	 * @todo Avoid business code in controller
 	 * @param mixed $setSize
 	 * @param mixed $size
 	 * @param mixed $sizeToQuery
@@ -672,7 +673,7 @@
 
 	/**
 	 * @dataProvider dataProviderIndex
 	 * @todo no work on controller
 	 * @param mixed $recipes
 	 * @param mixed $setKeywords
 	 * @param mixed $keywords
@@ -753,4 +754,34 @@
 			],
 		];
 	}
+
+	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());
+	}
 }