upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-05-28 04:32:30 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 25

fix: FilenameValidator::isForbidden should only check forbidden files

Browse source 
And not forbidden basenames as this is used for different purposes.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
This commit is contained in:
Ferdinand Thiessen 2024-08-12 18:11:31 +02:00 committed by Andy Scherzinger
parent e5a14f6831
commit 1e49c83556
2 changed files with 25 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
lib/private/Files/FilenameValidator.php
View file

@ -198,9 +198,7 @@ class FilenameValidator implements IFilenameValidator {
}
}
if ($this->isForbidden($filename)) {
throw new ReservedWordException();
}
$this->checkForbiddenName($filename);
$this->checkForbiddenExtension($filename);
@ -227,18 +225,25 @@ class FilenameValidator implements IFilenameValidator {
return true;
}
// Filename is not forbidden
return false;
}
protected function checkForbiddenName($filename): void {
if ($this->isForbidden($filename)) {
throw new ReservedWordException($this->l10n->t('"%1$s" is a forbidden file or folder name.', [$filename]));
}
// Check for forbidden basenames - basenames are the part of the file until the first dot
// (except if the dot is the first character as this is then part of the basename "hidden files")
$basename = substr($filename, 0, strpos($filename, '.', 1) ?: null);
$forbiddenNames = $this->getForbiddenBasenames();
if (in_array($basename, $forbiddenNames)) {
return true;
throw new ReservedWordException($this->l10n->t('"%1$s" is a forbidden prefix for file or folder names.', [$filename]));
}
// Filename is not forbidden
return false;
}
/**
* Check if a filename contains any of the forbidden characters
* @param string $filename
@ -252,7 +257,7 @@ class FilenameValidator implements IFilenameValidator {
foreach ($this->getForbiddenCharacters() as $char) {
if (str_contains($filename, $char)) {
throw new InvalidCharacterInPathException($this->l10n->t('Invalid character "%1$s" in filename', [$char]));
throw new InvalidCharacterInPathException($this->l10n->t('"%1$s" is not allowed inside a file or folder name.', [$char]));
}
}
}
@ -268,7 +273,11 @@ class FilenameValidator implements IFilenameValidator {
$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]));
if (str_starts_with($extension, '.')) {
throw new InvalidPathException($this->l10n->t('"%1$s" is a forbidden file type.', [$extension]));
} else {
throw new InvalidPathException($this->l10n->t('Filenames must not end with "%1$s".', [$extension]));
}
}
}
}

24
tests/lib/Files/FilenameValidatorTest.php
View file

@ -252,15 +252,13 @@ class FilenameValidatorTest extends TestCase {
/**
* @dataProvider dataIsForbidden
*/
public function testIsForbidden(string $filename, array $forbiddenNames, array $forbiddenBasenames, bool $expected): void {
public function testIsForbidden(string $filename, array $forbiddenNames, bool $expected): void {
/** @var FilenameValidator&MockObject */
$validator = $this->getMockBuilder(FilenameValidator::class)
->onlyMethods(['getForbiddenFilenames', 'getForbiddenBasenames'])
->onlyMethods(['getForbiddenFilenames'])
->setConstructorArgs([$this->l10n, $this->database, $this->config, $this->logger])
->getMock();
$validator->method('getForbiddenBasenames')
->willReturn($forbiddenBasenames);
$validator->method('getForbiddenFilenames')
->willReturn($forbiddenNames);
@ -270,27 +268,19 @@ class FilenameValidatorTest extends TestCase {
public function dataIsForbidden(): array {
return [
'valid name' => [
'a: b.txt', ['.htaccess'], [], false
'a: b.txt', ['.htaccess'], false
],
'valid name with some more parameters' => [
'a: b.txt', ['.htaccess'], [], false
'a: b.txt', ['.htaccess'], false
],
'valid name as only full forbidden should be matched' => [
'.htaccess.sample', ['.htaccess'], [], false,
'.htaccess.sample', ['.htaccess'], false,
],
'forbidden name' => [
'.htaccess', ['.htaccess'], [], true
'.htaccess', ['.htaccess'], true
],
'forbidden name - name is case insensitive' => [
'COM1', ['.htaccess', 'com1'], [], true,
],
'forbidden name - basename is checked' => [
// needed for Windows namespaces
'com1.suffix', ['.htaccess'], ['com1'], true
],
'forbidden name - basename is checked also with multiple extensions' => [
// needed for Windows namespaces
'com1.tar.gz', ['.htaccess'], ['com1'], true
'COM1', ['.htaccess', 'com1'], true,
],
];
}