From e151210a62a1fbe9fb885bd1dbb51315bd820e03 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Sun, 13 Jan 2013 14:33:19 +0100 Subject: [PATCH 1/4] Simplify the isSubDirectory() function isSubDirectory() checks if a specified $sub is a subdirectory of the $parent, this is needed to prevent file inclusions. Actually, the current code is more kind of a "hack" which I always struggle over if browsing through source. So this should be a much better implementation. The implementation is really straightforward: - [realpath()](http://php.net/manual/function.realpath.php) expands all symbolic links and resolves references to '/./', '/../' and extra '/' characters in the input path and return the canonicalized absolute pathname. - [strpos()](php.net/manual/function.strpos.php) returns FALSE if the substring wasn't found. Since this is an absolutely critical piece of code, I'd like to ensure that this is absolutely safe! --- lib/helper.php | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/lib/helper.php b/lib/helper.php index 1aba2a3810..a01743cc27 100644 --- a/lib/helper.php +++ b/lib/helper.php @@ -633,29 +633,9 @@ class OC_Helper { * @return bool */ public static function issubdirectory($sub, $parent) { - if($sub == null || $sub == '' || $parent == null || $parent == '') { - return false; + if (strpos(realpath($sub), realpath($parent)) !== false) { + return true; } - $realpath_sub = realpath($sub); - $realpath_parent = realpath($parent); - if(($realpath_sub == false && substr_count($realpath_sub, './') != 0) || ($realpath_parent == false && substr_count($realpath_parent, './') != 0)) { //it checks for both ./ and ../ - return false; - } - if($realpath_sub && $realpath_sub != '' && $realpath_parent && $realpath_parent != '') { - if(substr($realpath_sub, 0, strlen($realpath_parent)) == $realpath_parent) { - return true; - } - }else{ - if(substr($sub, 0, strlen($parent)) == $parent) { - return true; - } - } - /*echo 'SUB: ' . $sub . "\n"; - echo 'PAR: ' . $parent . "\n"; - echo 'REALSUB: ' . $realpath_sub . "\n"; - echo 'REALPAR: ' . $realpath_parent . "\n"; - echo substr($realpath_sub, 0, strlen($realpath_parent)); - exit;*/ return false; } From c27833b1435998a1444b53180c42ef10c47c818b Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Sun, 13 Jan 2013 14:50:31 +0100 Subject: [PATCH 2/4] Add @brief to description --- lib/helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/helper.php b/lib/helper.php index a01743cc27..7dd53411fa 100644 --- a/lib/helper.php +++ b/lib/helper.php @@ -626,7 +626,7 @@ class OC_Helper { } /* - * checks if $sub is a subdirectory of $parent + * @brief Checks if $sub is a subdirectory of $parent * * @param string $sub * @param string $parent From b7db967dc5c80b7b62c0a734d174acdc0fa49f6d Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Sun, 13 Jan 2013 14:54:18 +0100 Subject: [PATCH 3/4] Commentblocks should begin with two * --- lib/helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/helper.php b/lib/helper.php index 7dd53411fa..47292c56ea 100644 --- a/lib/helper.php +++ b/lib/helper.php @@ -625,7 +625,7 @@ class OC_Helper { return $newpath; } - /* + /** * @brief Checks if $sub is a subdirectory of $parent * * @param string $sub From 99adfbdb8663d89b650feab6b797f344a46c82f3 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 14 Jan 2013 16:51:31 +0100 Subject: [PATCH 4/4] Check for string position instead of string existence otherwise /foo/bar would be detected as a subfolder of /bar THX @icewind1991 --- lib/helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/helper.php b/lib/helper.php index 47292c56ea..e7c9ac8015 100644 --- a/lib/helper.php +++ b/lib/helper.php @@ -633,7 +633,7 @@ class OC_Helper { * @return bool */ public static function issubdirectory($sub, $parent) { - if (strpos(realpath($sub), realpath($parent)) !== false) { + if (strpos(realpath($sub), realpath($parent)) === 0) { return true; } return false;