Merge pull request #46371 from nextcloud/feat/filename-validator

feat: Add `IFilenameValidator` to have one consistent place for filename validation

config/config.sample.php

@@ -1976,26 +1976,38 @@ $CONFIG = [
 'updatedirectory' => '',
 
 /**
- * Blacklist a specific file or files and disallow the upload of files
+ * Block a specific file or files and disallow the upload of files
  * with this name. ``.htaccess`` is blocked by default.
+ *
  * WARNING: USE THIS ONLY IF YOU KNOW WHAT YOU ARE DOING.
  *
+ * Note that this list is case-insensitive.
+ *
  * Defaults to ``array('.htaccess')``
  */
-'blacklisted_files' => ['.htaccess'],
+'forbidden_filenames' => ['.htaccess'],
 
 /**
- * Blacklist characters from being used in filenames. This is useful if you
+ * Block characters from being used in filenames. This is useful if you
  * have a filesystem or OS which does not support certain characters like windows.
  *
- * The '/' and '\' characters are always forbidden.
+ * The '/' and '\' characters are always forbidden, as well as all characters in the ASCII range [0-31].
  *
- * Example for windows systems: ``array('?', '<', '>', ':', '*', '|', '"', chr(0), "\n", "\r")``
+ * Example for windows systems: ``array('?', '<', '>', ':', '*', '|', '"')``
  * see https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits
  *
  * Defaults to ``array()``
  */
-'forbidden_chars' => [],
+'forbidden_filename_characters' => [],
+
+/**
+ * Deny extensions from being used for filenames.
+ * 
+ * The '.part' extension is always forbidden, as this is used internally by Nextcloud.
+ * 
+ * Defaults to ``array('.filepart', '.part')``
+ */
+'forbidden_filename_extensions' => ['.part', '.filepart'],
 
 /**
  * If you are applying a theme to Nextcloud, enter the name of the theme here.

lib/composer/composer/autoload_classmap.php

@@ -379,6 +379,7 @@ return array(
     'OCP\\Files\\ForbiddenException' => $baseDir . '/lib/public/Files/ForbiddenException.php',
     'OCP\\Files\\GenericFileException' => $baseDir . '/lib/public/Files/GenericFileException.php',
     'OCP\\Files\\IAppData' => $baseDir . '/lib/public/Files/IAppData.php',
+    'OCP\\Files\\IFilenameValidator' => $baseDir . '/lib/public/Files/IFilenameValidator.php',
     'OCP\\Files\\IHomeStorage' => $baseDir . '/lib/public/Files/IHomeStorage.php',
     'OCP\\Files\\IMimeTypeDetector' => $baseDir . '/lib/public/Files/IMimeTypeDetector.php',
     'OCP\\Files\\IMimeTypeLoader' => $baseDir . '/lib/public/Files/IMimeTypeLoader.php',
@@ -1445,6 +1446,7 @@ return array(
     'OC\\Files\\Config\\UserMountCache' => $baseDir . '/lib/private/Files/Config/UserMountCache.php',
     'OC\\Files\\Config\\UserMountCacheListener' => $baseDir . '/lib/private/Files/Config/UserMountCacheListener.php',
     'OC\\Files\\FileInfo' => $baseDir . '/lib/private/Files/FileInfo.php',
+    'OC\\Files\\FilenameValidator' => $baseDir . '/lib/private/Files/FilenameValidator.php',
     'OC\\Files\\Filesystem' => $baseDir . '/lib/private/Files/Filesystem.php',
     'OC\\Files\\Lock\\LockManager' => $baseDir . '/lib/private/Files/Lock/LockManager.php',
     'OC\\Files\\Mount\\CacheMountProvider' => $baseDir . '/lib/private/Files/Mount/CacheMountProvider.php',

lib/composer/composer/autoload_static.php

@@ -412,6 +412,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OCP\\Files\\ForbiddenException' => __DIR__ . '/../../..' . '/lib/public/Files/ForbiddenException.php',
         'OCP\\Files\\GenericFileException' => __DIR__ . '/../../..' . '/lib/public/Files/GenericFileException.php',
         'OCP\\Files\\IAppData' => __DIR__ . '/../../..' . '/lib/public/Files/IAppData.php',
+        'OCP\\Files\\IFilenameValidator' => __DIR__ . '/../../..' . '/lib/public/Files/IFilenameValidator.php',
         'OCP\\Files\\IHomeStorage' => __DIR__ . '/../../..' . '/lib/public/Files/IHomeStorage.php',
         'OCP\\Files\\IMimeTypeDetector' => __DIR__ . '/../../..' . '/lib/public/Files/IMimeTypeDetector.php',
         'OCP\\Files\\IMimeTypeLoader' => __DIR__ . '/../../..' . '/lib/public/Files/IMimeTypeLoader.php',
@@ -1478,6 +1479,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Files\\Config\\UserMountCache' => __DIR__ . '/../../..' . '/lib/private/Files/Config/UserMountCache.php',
         'OC\\Files\\Config\\UserMountCacheListener' => __DIR__ . '/../../..' . '/lib/private/Files/Config/UserMountCacheListener.php',
         'OC\\Files\\FileInfo' => __DIR__ . '/../../..' . '/lib/private/Files/FileInfo.php',
+        'OC\\Files\\FilenameValidator' => __DIR__ . '/../../..' . '/lib/private/Files/FilenameValidator.php',
         'OC\\Files\\Filesystem' => __DIR__ . '/../../..' . '/lib/private/Files/Filesystem.php',
         'OC\\Files\\Lock\\LockManager' => __DIR__ . '/../../..' . '/lib/private/Files/Lock/LockManager.php',
         'OC\\Files\\Mount\\CacheMountProvider' => __DIR__ . '/../../..' . '/lib/private/Files/Mount/CacheMountProvider.php',

lib/private/Files/FilenameValidator.php

@@ -0,0 +1,249 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OC\Files;
+
+use OCP\Files\EmptyFileNameException;
+use OCP\Files\FileNameTooLongException;
+use OCP\Files\IFilenameValidator;
+use OCP\Files\InvalidCharacterInPathException;
+use OCP\Files\InvalidPathException;
+use OCP\Files\ReservedWordException;
+use OCP\IConfig;
+use OCP\IL10N;
+use OCP\L10N\IFactory;
+use Psr\Log\LoggerInterface;
+
+/**
+ * @since 30.0.0
+ */
+class FilenameValidator implements IFilenameValidator {
+
+	private IL10N $l10n;
+
+	/**
+	 * @var list<string>
+	 */
+	private array $forbiddenNames = [];
+
+	/**
+	 * @var list<string>
+	 */
+	private array $forbiddenCharacters = [];
+
+	/**
+	 * @var list<string>
+	 */
+	private array $forbiddenExtensions = [];
+
+	public function __construct(
+		IFactory $l10nFactory,
+		private IConfig $config,
+		private LoggerInterface $logger,
+	) {
+		$this->l10n = $l10nFactory->get('core');
+	}
+
+	/**
+	 * Get a list of reserved filenames that must not be used
+	 * This list should be checked case-insensitive, all names are returned lowercase.
+	 * @return list<string>
+	 * @since 30.0.0
+	 */
+	public function getForbiddenExtensions(): array {
+		if (empty($this->forbiddenExtensions)) {
+			$forbiddenExtensions = $this->config->getSystemValue('forbidden_filename_extensions', ['.filepart']);
+			if (!is_array($forbiddenExtensions)) {
+				$this->logger->error('Invalid system config value for "forbidden_filename_extensions" is ignored.');
+				$forbiddenExtensions = ['.filepart'];
+			}
+
+			// Always forbid .part files as they are used internally
+			$forbiddenExtensions = array_merge($forbiddenExtensions, ['.part']);
+
+			// The list is case insensitive so we provide it always lowercase
+			$forbiddenExtensions = array_map('mb_strtolower', $forbiddenExtensions);
+			$this->forbiddenExtensions = array_values($forbiddenExtensions);
+		}
+		return $this->forbiddenExtensions;
+	}
+
+	/**
+	 * Get a list of forbidden filename extensions that must not be used
+	 * This list should be checked case-insensitive, all names are returned lowercase.
+	 * @return list<string>
+	 * @since 30.0.0
+	 */
+	public function getForbiddenFilenames(): array {
+		if (empty($this->forbiddenNames)) {
+			$forbiddenNames = $this->config->getSystemValue('forbidden_filenames', ['.htaccess']);
+			if (!is_array($forbiddenNames)) {
+				$this->logger->error('Invalid system config value for "forbidden_filenames" is ignored.');
+				$forbiddenNames = ['.htaccess'];
+			}
+
+			// Handle legacy config option
+			// TODO: Drop with Nextcloud 34
+			$legacyForbiddenNames = $this->config->getSystemValue('blacklisted_files', []);
+			if (!is_array($legacyForbiddenNames)) {
+				$this->logger->error('Invalid system config value for "blacklisted_files" is ignored.');
+				$legacyForbiddenNames = [];
+			}
+			if (!empty($legacyForbiddenNames)) {
+				$this->logger->warning('System config option "blacklisted_files" is deprecated and will be removed in Nextcloud 34, use "forbidden_filenames" instead.');
+			}
+			$forbiddenNames = array_merge($legacyForbiddenNames, $forbiddenNames);
+
+			// The list is case insensitive so we provide it always lowercase
+			$forbiddenNames = array_map('mb_strtolower', $forbiddenNames);
+			$this->forbiddenNames = array_values($forbiddenNames);
+		}
+		return $this->forbiddenNames;
+	}
+
+	/**
+	 * Get a list of characters forbidden in filenames
+	 *
+	 * Note: Characters in the range [0-31] are always forbidden,
+	 * even if not inside this list (see OCP\Files\Storage\IStorage::verifyPath).
+	 *
+	 * @return list<string>
+	 * @since 30.0.0
+	 */
+	public function getForbiddenCharacters(): array {
+		if (empty($this->forbiddenCharacters)) {
+			// Get always forbidden characters
+			$forbiddenCharacters = str_split(\OCP\Constants::FILENAME_INVALID_CHARS);
+			if ($forbiddenCharacters === false) {
+				$forbiddenCharacters = [];
+			}
+
+			// Get admin defined invalid characters
+			$additionalChars = $this->config->getSystemValue('forbidden_filename_characters', []);
+			if (!is_array($additionalChars)) {
+				$this->logger->error('Invalid system config value for "forbidden_filename_characters" is ignored.');
+				$additionalChars = [];
+			}
+			$forbiddenCharacters = array_merge($forbiddenCharacters, $additionalChars);
+
+			// Handle legacy config option
+			// TODO: Drop with Nextcloud 34
+			$legacyForbiddenCharacters = $this->config->getSystemValue('forbidden_chars', []);
+			if (!is_array($legacyForbiddenCharacters)) {
+				$this->logger->error('Invalid system config value for "forbidden_chars" is ignored.');
+				$legacyForbiddenCharacters = [];
+			}
+			if (!empty($legacyForbiddenCharacters)) {
+				$this->logger->warning('System config option "forbidden_chars" is deprecated and will be removed in Nextcloud 34, use "forbidden_filename_characters" instead.');
+			}
+			$forbiddenCharacters = array_merge($legacyForbiddenCharacters, $forbiddenCharacters);
+
+			$this->forbiddenCharacters = array_values($forbiddenCharacters);
+		}
+		return $this->forbiddenCharacters;
+	}
+
+	/**
+	 * @inheritdoc
+	 */
+	public function isFilenameValid(string $filename): bool {
+		try {
+			$this->validateFilename($filename);
+		} catch (\OCP\Files\InvalidPathException) {
+			return false;
+		}
+		return true;
+	}
+
+	/**
+	 * @inheritdoc
+	 */
+	public function validateFilename(string $filename): void {
+		$trimmed = trim($filename);
+		if ($trimmed === '') {
+			throw new EmptyFileNameException();
+		}
+
+		// the special directories . and .. would cause never ending recursion
+		if ($trimmed === '.' || $trimmed === '..') {
+			throw new ReservedWordException();
+		}
+
+		// 255 characters is the limit on common file systems (ext/xfs)
+		// oc_filecache has a 250 char length limit for the filename
+		if (isset($filename[250])) {
+			throw new FileNameTooLongException();
+		}
+
+		if ($this->isForbidden($filename)) {
+			throw new ReservedWordException();
+		}
+
+		$this->checkForbiddenExtension($filename);
+
+		$this->checkForbiddenCharacters($filename);
+	}
+
+	/**
+	 * Check if the filename is forbidden
+	 * @param string $path Path to check the filename
+	 * @return bool True if invalid name, False otherwise
+	 */
+	public function isForbidden(string $path): bool {
+		$filename = basename($path);
+		$filename = mb_strtolower($filename);
+
+		if ($filename === '') {
+			return false;
+		}
+
+		// The name part without extension
+		$basename = substr($filename, 0, strpos($filename, '.', 1) ?: null);
+		// Check for forbidden filenames
+		$forbiddenNames = $this->getForbiddenFilenames();
+		if (in_array($basename, $forbiddenNames)) {
+			return true;
+		}
+
+		// Filename is not forbidden
+		return false;
+	}
+
+	/**
+	 * Check if a filename contains any of the forbidden characters
+	 * @param string $filename
+	 * @throws InvalidCharacterInPathException
+	 */
+	protected function checkForbiddenCharacters(string $filename): void {
+		$sanitizedFileName = filter_var($filename, FILTER_UNSAFE_RAW, FILTER_FLAG_STRIP_LOW);
+		if ($sanitizedFileName !== $filename) {
+			throw new InvalidCharacterInPathException();
+		}
+
+		foreach ($this->getForbiddenCharacters() as $char) {
+			if (str_contains($filename, $char)) {
+				throw new InvalidCharacterInPathException($char);
+			}
+		}
+	}
+
+	/**
+	 * Check if a filename has a forbidden filename extension
+	 * @param string $filename The filename to validate
+	 * @throws InvalidPathException
+	 */
+	protected function checkForbiddenExtension(string $filename): void {
+		$filename = mb_strtolower($filename);
+		// Check for forbidden filename exten<sions
+		$forbiddenExtensions = $this->getForbiddenExtensions();
+		foreach ($forbiddenExtensions as $extension) {
+			if (str_ends_with($filename, $extension)) {
+				throw new InvalidPathException($this->l10n->t('Invalid filename extension "%1$s"', [$extension]));
+			}
+		}
+	}
+};

lib/private/Server.php

@@ -1371,6 +1371,8 @@ class Server extends ServerContainer implements IServerContainer {
 
 		$this->registerAlias(\OCP\Files\AppData\IAppDataFactory::class, \OC\Files\AppData\Factory::class);
 
+		$this->registerAlias(\OCP\Files\IFilenameValidator::class, \OC\Files\FilenameValidator::class);
+
 		$this->registerAlias(IBinaryFinder::class, BinaryFinder::class);
 
 		$this->registerAlias(\OCP\Share\IPublicShareTemplateFactory::class, \OC\Share20\PublicShareTemplateFactory::class);

lib/public/Files/IFilenameValidator.php

@@ -0,0 +1,39 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OCP\Files;
+
+/**
+ * @since 30.0.0
+ */
+interface IFilenameValidator {
+
+	/**
+	 * It is recommended to use `\OCP\Files\Storage\IStorage::isFileValid` instead as this
+	 * only checks if the filename is valid in general but not for a specific storage
+	 * which might have additional naming rules.
+	 *
+	 * @param string $filename The filename to check for validity
+	 * @return bool
+	 * @since 30.0.0
+	 */
+	public function isFilenameValid(string $filename): bool;
+
+	/**
+	 * It is recommended to use `\OCP\Files\Storage\IStorage::isFileValid` instead as this
+	 * only checks if the filename is valid in general but not for a specific storage
+	 * which might have additional naming rules.
+	 *
+	 * This will validate a filename and throw an exception with details on error.
+	 *
+	 * @param string $filename The filename to check for validity
+	 * @throws \OCP\Files\InvalidPathException or one of its child classes in case of an error
+	 * @since 30.0.0
+	 */
+	public function validateFilename(string $filename): void;
+}

tests/lib/Files/FilenameValidatorTest.php

@@ -0,0 +1,188 @@
+<?php
+
+declare(strict_types=1);
+
+/*!
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace Test\Files;
+
+use OC\Files\FilenameValidator;
+use OCP\Files\EmptyFileNameException;
+use OCP\Files\FileNameTooLongException;
+use OCP\Files\InvalidCharacterInPathException;
+use OCP\Files\InvalidPathException;
+use OCP\Files\ReservedWordException;
+use OCP\IConfig;
+use OCP\IL10N;
+use OCP\L10N\IFactory;
+use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
+use Test\TestCase;
+
+class FilenameValidatorTest extends TestCase {
+
+	protected IFactory&MockObject $l10n;
+	protected IConfig&MockObject $config;
+	protected LoggerInterface&MockObject $logger;
+
+	protected function setUp(): void {
+		parent::setUp();
+		$l10n = $this->createMock(IL10N::class);
+		$l10n->method('t')
+			->willReturnCallback(fn ($string, $params) => sprintf($string, ...$params));
+		$this->l10n = $this->createMock(IFactory::class);
+		$this->l10n
+			->method('get')
+			->with('core')
+			->willReturn($l10n);
+
+		$this->config = $this->createMock(IConfig::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
+	}
+
+	/**
+	 * @dataProvider dataValidateFilename
+	 */
+	public function testValidateFilename(
+		string $filename,
+		array $forbiddenNames,
+		array $forbiddenExtensions,
+		array $forbiddenCharacters,
+		?string $exception,
+	): void {
+		/** @var FilenameValidator&MockObject */
+		$validator = $this->getMockBuilder(FilenameValidator::class)
+			->onlyMethods(['getForbiddenExtensions', 'getForbiddenFilenames', 'getForbiddenCharacters'])
+			->setConstructorArgs([$this->l10n, $this->config, $this->logger])
+			->getMock();
+
+		$validator->method('getForbiddenCharacters')
+			->willReturn($forbiddenCharacters);
+		$validator->method('getForbiddenExtensions')
+			->willReturn($forbiddenExtensions);
+		$validator->method('getForbiddenFilenames')
+			->willReturn($forbiddenNames);
+
+		if ($exception !== null) {
+			$this->expectException($exception);
+		} else {
+			$this->expectNotToPerformAssertions();
+		}
+		$validator->validateFilename($filename);
+	}
+
+	/**
+	 * @dataProvider dataValidateFilename
+	 */
+	public function testIsFilenameValid(
+		string $filename,
+		array $forbiddenNames,
+		array $forbiddenExtensions,
+		array $forbiddenCharacters,
+		?string $exception,
+	): void {
+		/** @var FilenameValidator&MockObject */
+		$validator = $this->getMockBuilder(FilenameValidator::class)
+			->onlyMethods(['getForbiddenExtensions', 'getForbiddenFilenames', 'getForbiddenCharacters'])
+			->setConstructorArgs([$this->l10n, $this->config, $this->logger])
+			->getMock();
+
+		$validator->method('getForbiddenCharacters')
+			->willReturn($forbiddenCharacters);
+		$validator->method('getForbiddenExtensions')
+			->willReturn($forbiddenExtensions);
+		$validator->method('getForbiddenFilenames')
+			->willReturn($forbiddenNames);
+
+
+		$this->assertEquals($exception === null, $validator->isFilenameValid($filename));
+	}
+
+	public function dataValidateFilename(): array {
+		return [
+			'valid name' => [
+				'a: b.txt', ['.htaccess'], [], [], null
+			],
+			'valid name with some more parameters' => [
+				'a: b.txt', ['.htaccess'], ['exe'], ['~'], null
+			],
+			'forbidden name' => [
+				'.htaccess', ['.htaccess'], [], [], ReservedWordException::class
+			],
+			'forbidden name - name is case insensitive' => [
+				'COM1', ['.htaccess', 'com1'], [], [], ReservedWordException::class
+			],
+			'forbidden name - name checks the filename' => [
+				// needed for Windows namespaces
+				'com1.suffix', ['.htaccess', 'com1'], [], [], ReservedWordException::class
+			],
+			'invalid character' => [
+				'a: b.txt', ['.htaccess'], [], [':'], InvalidCharacterInPathException::class
+			],
+			'invalid path' => [
+				'../../foo.bar', ['.htaccess'], [], ['/', '\\'], InvalidCharacterInPathException::class,
+			],
+			'invalid extension' => [
+				'a: b.txt', ['.htaccess'], ['.txt'], [], InvalidPathException::class
+			],
+			'empty filename' => [
+				'', [], [], [], EmptyFileNameException::class
+			],
+			'reserved unix name "."' => [
+				'.', [], [], [], InvalidPathException::class
+			],
+			'reserved unix name ".."' => [
+				'..', [], [], [], ReservedWordException::class
+			],
+			'too long filename "."' => [
+				str_repeat('a', 251), [], [], [], FileNameTooLongException::class
+			],
+			// make sure to not split the list entries as they migh contain Unicode sequences
+			// in this example the "face in clouds" emoji contains the clouds emoji so only having clouds is ok
+			['🌫️.txt', ['.htaccess'], [], ['😶‍🌫️'], null],
+			// This is the reverse: clouds are forbidden -> so is also the face in the clouds emoji
+			['😶‍🌫️.txt', ['.htaccess'], [], ['🌫️'], InvalidCharacterInPathException::class],
+		];
+	}
+
+	/**
+	 * @dataProvider dataIsForbidden
+	 */
+	public function testIsForbidden(string $filename, array $forbiddenNames, bool $expected): void {
+		/** @var FilenameValidator&MockObject */
+		$validator = $this->getMockBuilder(FilenameValidator::class)
+			->onlyMethods(['getForbiddenFilenames'])
+			->setConstructorArgs([$this->l10n, $this->config, $this->logger])
+			->getMock();
+
+		$validator->method('getForbiddenFilenames')
+			->willReturn($forbiddenNames);
+
+
+		$this->assertEquals($expected, $validator->isFilenameValid($filename));
+	}
+
+	public function dataIsForbidden(): array {
+		return [
+			'valid name' => [
+				'a: b.txt', ['.htaccess'], true
+			],
+			'valid name with some more parameters' => [
+				'a: b.txt', ['.htaccess'], true
+			],
+			'forbidden name' => [
+				'.htaccess', ['.htaccess'], false
+			],
+			'forbidden name - name is case insensitive' => [
+				'COM1', ['.htaccess', 'com1'], false
+			],
+			'forbidden name - name checks the filename' => [
+				// needed for Windows namespaces
+				'com1.suffix', ['.htaccess', 'com1'], false
+			],
+		];
+	}
+}