Commit e8d9c8d1 authored by Benni Mack's avatar Benni Mack Committed by Oliver Hader
Browse files

[SECURITY] Disallow invalid encoding in GeneralUtility::validPathStr

Directory names, which have an invalid UTF encoding,
cause the preg_match() to return false.
To avoid that the complete statement in GeneralUtility::validPathStr()
returns true in this case, a strict comparison against 0 is added,
so that we ensure that strings with invalid encodings are rejected
by this API method.

As a consequence UTF-16 encoded path names are rejected as well, if the
system / file system does not support them.

Resolves: #73453
Releases: master, 8.4, 7.6, 6.2
Security-Commit: c54aa56d18815aa1867ec54358ad419ea03ec205
Security-Bulletins: TYPO3-CORE-SA-2016-023, 024
Change-Id: Iedd6628050d8cdf2efe429bcd7b577f5a6d11805
Reviewed-on: https://review.typo3.org/50744

Reviewed-by: Oliver Hader's avatarOliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader's avatarOliver Hader <oliver.hader@typo3.org>
parent efb1443e
......@@ -57,7 +57,8 @@
"se/selenium-server-standalone": "~2.53",
"7elix/styleguide": "~8.0.0",
"friendsofphp/php-cs-fixer": "^1.12",
"fiunchinho/phpunit-randomizer": "~2.0.3"
"fiunchinho/phpunit-randomizer": "~2.0.3",
"symfony/polyfill-mbstring": "~1.0"
},
"suggest": {
"ext-gd": "GDlib/Freetype is required for building images with text (GIFBUILDER) and can also be used to scale images",
......
......@@ -4,8 +4,8 @@
"Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file",
"This file is @generated automatically"
],
"hash": "06e0af32c944e8ee5ac81f3a95a29261",
"content-hash": "16442457bdb44cb99ea5714900b0dd5c",
"hash": "90479fd517e730794243c516ef4ba9ae",
"content-hash": "9ac01b16aca6a55472ecf320b6dacf31",
"packages": [
{
"name": "cogpowered/finediff",
......
......@@ -3314,12 +3314,11 @@ class GeneralUtility
* @param string $theFile File path to evaluate
* @return bool TRUE, $theFile is allowed path string, FALSE otherwise
* @see http://php.net/manual/en/security.filesystem.nullbytes.php
* @todo Possible improvement: Should it rawurldecode the string first to check if any of these characters is encoded?
*/
public static function validPathStr($theFile)
{
return strpos($theFile, '//') === false && strpos($theFile, '\\') === false
&& !preg_match('#(?:^\\.\\.|/\\.\\./|[[:cntrl:]])#u', $theFile);
&& preg_match('#(?:^\\.\\.|/\\.\\./|[[:cntrl:]])#u', $theFile) === 0;
}
/**
......
......@@ -4294,14 +4294,30 @@ class GeneralUtilityTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
*/
public function validPathStrInvalidCharactersDataProvider()
{
return [
$data = [
'double slash in path' => ['path//path'],
'backslash in path' => ['path\\path'],
'directory up in path' => ['path/../path'],
'directory up at the beginning' => ['../path'],
'NUL character in path' => ['path' . chr(0) . 'path'],
'BS character in path' => ['path' . chr(8) . 'path']
'BS character in path' => ['path' . chr(8) . 'path'],
'invalid UTF-8-sequence' => ["\xc0" . 'path/path'],
'Could be overlong NUL in some UTF-8 implementations, invalid in RFC3629' => ["\xc0\x80" . 'path/path'],
];
// Mixing with regular utf-8
$utf8Characters = 'Ссылка/';
foreach ($data as $key => $value) {
$data[$key . ' with UTF-8 characters prepended'] = [$utf8Characters . $value[0]];
$data[$key . ' with UTF-8 characters appended'] = [$value[0] . $utf8Characters];
}
// Encoding with UTF-16
foreach ($data as $key => $value) {
$data[$key . ' encoded with UTF-16'] = [mb_convert_encoding($value[0], 'UTF-16')];
}
return $data;
}
/**
......@@ -4316,14 +4332,28 @@ class GeneralUtilityTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
$this->assertFalse(GeneralUtility::validPathStr($path));
}
/**
* Data provider for positive values within validPathStr()
*/
public function validPathStrDataProvider()
{
$data = [
'normal ascii path' => ['fileadmin/templates/myfile..xml'],
'special character' => ['fileadmin/templates/Ссылка (fce).xml']
];
return $data;
}
/**
* Tests whether Unicode characters are recognized as valid file name characters.
*
* @dataProvider validPathStrDataProvider
* @test
*/
public function validPathStrWorksWithUnicodeFileNames()
public function validPathStrWorksWithUnicodeFileNames($path)
{
$this->assertTrue(GeneralUtility::validPathStr('fileadmin/templates/Ссылка (fce).xml'));
$this->assertTrue(GeneralUtility::validPathStr($path));
}
/**
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment