From a40e49cae5983d8158562e142919cd3108bd2fd8 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Sun, 11 May 2014 15:49:19 +0200 Subject: [PATCH 1/2] Harden issubdirectory() realpath() may return false in case the directory does not exist since we can not be sure how different PHP versions may behave here we do an additional check whether realpath returned false --- lib/private/helper.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/private/helper.php b/lib/private/helper.php index 64da1f6fb1..1883ae2a8f 100644 --- a/lib/private/helper.php +++ b/lib/private/helper.php @@ -733,9 +733,21 @@ class OC_Helper { * @return bool */ public static function issubdirectory($sub, $parent) { - if (strpos(realpath($sub), realpath($parent)) === 0) { + $realpathSub = realpath($sub); + $realpathParent = realpath($parent); + + // realpath() may return false in case the directory does not exist + // since we can not be sure how different PHP versions may behave here + // we do an additional check whether realpath returned false + if($realpathSub === false || $realpathParent === false) { + return false; + } + + // Check whether $sub is a subdirectory of $parent + if (strpos($realpathSub, $realpathParent) === 0) { return true; } + return false; } From fd5b2d11d6a174f46df563917060e350f6df079a Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Sun, 11 May 2014 15:50:59 +0200 Subject: [PATCH 2/2] Rename issubdirectory to isSubDirectory --- lib/base.php | 2 +- lib/private/helper.php | 2 +- lib/private/l10n.php | 10 +++++----- tests/lib/helper.php | 8 ++++---- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/base.php b/lib/base.php index 1f7d0c0da6..f8266ac649 100644 --- a/lib/base.php +++ b/lib/base.php @@ -613,7 +613,7 @@ class OC { if (!is_null(self::$REQUESTEDFILE)) { $subdir = OC_App::getAppPath(OC::$REQUESTEDAPP) . '/' . self::$REQUESTEDFILE; $parent = OC_App::getAppPath(OC::$REQUESTEDAPP); - if (!OC_Helper::issubdirectory($subdir, $parent)) { + if (!OC_Helper::isSubDirectory($subdir, $parent)) { self::$REQUESTEDFILE = null; header('HTTP/1.0 404 Not Found'); exit; diff --git a/lib/private/helper.php b/lib/private/helper.php index 1883ae2a8f..6bc054bce8 100644 --- a/lib/private/helper.php +++ b/lib/private/helper.php @@ -732,7 +732,7 @@ class OC_Helper { * @param string $parent * @return bool */ - public static function issubdirectory($sub, $parent) { + public static function isSubDirectory($sub, $parent) { $realpathSub = realpath($sub); $realpathParent = realpath($parent); diff --git a/lib/private/l10n.php b/lib/private/l10n.php index d6680d6344..c1596a6416 100644 --- a/lib/private/l10n.php +++ b/lib/private/l10n.php @@ -134,10 +134,10 @@ class OC_L10N implements \OCP\IL10N { $i18ndir = self::findI18nDir($app); // Localization is in /l10n, Texts are in $i18ndir // (Just no need to define date/time format etc. twice) - if((OC_Helper::issubdirectory($i18ndir.$lang.'.php', OC::$SERVERROOT.'/core/l10n/') - || OC_Helper::issubdirectory($i18ndir.$lang.'.php', OC::$SERVERROOT.'/lib/l10n/') - || OC_Helper::issubdirectory($i18ndir.$lang.'.php', OC::$SERVERROOT.'/settings') - || OC_Helper::issubdirectory($i18ndir.$lang.'.php', OC_App::getAppPath($app).'/l10n/') + if((OC_Helper::isSubDirectory($i18ndir.$lang.'.php', OC::$SERVERROOT.'/core/l10n/') + || OC_Helper::isSubDirectory($i18ndir.$lang.'.php', OC::$SERVERROOT.'/lib/l10n/') + || OC_Helper::isSubDirectory($i18ndir.$lang.'.php', OC::$SERVERROOT.'/settings') + || OC_Helper::isSubDirectory($i18ndir.$lang.'.php', OC_App::getAppPath($app).'/l10n/') ) && file_exists($i18ndir.$lang.'.php')) { // Include the file, save the data from $CONFIG @@ -162,7 +162,7 @@ class OC_L10N implements \OCP\IL10N { } } - if(file_exists(OC::$SERVERROOT.'/core/l10n/l10n-'.$lang.'.php') && OC_Helper::issubdirectory(OC::$SERVERROOT.'/core/l10n/l10n-'.$lang.'.php', OC::$SERVERROOT.'/core/l10n/')) { + if(file_exists(OC::$SERVERROOT.'/core/l10n/l10n-'.$lang.'.php') && OC_Helper::isSubDirectory(OC::$SERVERROOT.'/core/l10n/l10n-'.$lang.'.php', OC::$SERVERROOT.'/core/l10n/')) { // Include the file, save the data from $CONFIG include OC::$SERVERROOT.'/core/l10n/l10n-'.$lang.'.php'; if(isset($LOCALIZATIONS) && is_array($LOCALIZATIONS)) { diff --git a/tests/lib/helper.php b/tests/lib/helper.php index 5663df7c9a..59db30a73f 100644 --- a/tests/lib/helper.php +++ b/tests/lib/helper.php @@ -120,15 +120,15 @@ class Test_Helper extends PHPUnit_Framework_TestCase { $this->assertEquals($result, $expected); } - function testIssubdirectory() { - $result = OC_Helper::issubdirectory("./data/", "/anotherDirectory/"); + function testIsSubDirectory() { + $result = OC_Helper::isSubDirectory("./data/", "/anotherDirectory/"); $this->assertFalse($result); - $result = OC_Helper::issubdirectory("./data/", "./data/"); + $result = OC_Helper::isSubDirectory("./data/", "./data/"); $this->assertTrue($result); mkdir("data/TestSubdirectory", 0777); - $result = OC_Helper::issubdirectory("data/TestSubdirectory/", "data"); + $result = OC_Helper::isSubDirectory("data/TestSubdirectory/", "data"); rmdir("data/TestSubdirectory"); $this->assertTrue($result); }