From ab79caf29bf4173e15f9dad00a206c6bb1f3999c Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 25 Aug 2014 14:06:27 +0200 Subject: [PATCH 1/4] Check if the parent is writable to check if a file is deletable --- lib/private/files/storage/common.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index 4799c86514..4680971b27 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -95,7 +95,11 @@ abstract class Common implements \OC\Files\Storage\Storage { } public function isDeletable($path) { - return $this->isUpdatable($path); + if ($path === '' or $path === '/') { + return false; + } + $parent = dirname($path); + return $this->isUpdatable($parent); } public function isSharable($path) { From d25a9a118f7824a72f193d57373355e51323f118 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 25 Aug 2014 14:06:48 +0200 Subject: [PATCH 2/4] Check if a folder is deletable before we try to recursively delete it --- apps/files_external/lib/google.php | 3 +++ apps/files_external/lib/streamwrapper.php | 2 +- apps/files_external/lib/swift.php | 2 +- lib/private/files/storage/local.php | 3 +++ lib/private/files/storage/mappedlocal.php | 3 +++ 5 files changed, 11 insertions(+), 2 deletions(-) diff --git a/apps/files_external/lib/google.php b/apps/files_external/lib/google.php index 88d82d51e2..5d238a363d 100644 --- a/apps/files_external/lib/google.php +++ b/apps/files_external/lib/google.php @@ -204,6 +204,9 @@ class Google extends \OC\Files\Storage\Common { } public function rmdir($path) { + if (!$this->isDeletable($path)) { + return false; + } if (trim($path, '/') === '') { $dir = $this->opendir($path); if(is_resource($dir)) { diff --git a/apps/files_external/lib/streamwrapper.php b/apps/files_external/lib/streamwrapper.php index 44bd9a0161..adccb2919e 100644 --- a/apps/files_external/lib/streamwrapper.php +++ b/apps/files_external/lib/streamwrapper.php @@ -21,7 +21,7 @@ abstract class StreamWrapper extends Common { } public function rmdir($path) { - if ($this->file_exists($path)) { + if ($this->file_exists($path) and $this->isDeletable($path)) { $dh = $this->opendir($path); while (($file = readdir($dh)) !== false) { if ($this->is_dir($path . '/' . $file)) { diff --git a/apps/files_external/lib/swift.php b/apps/files_external/lib/swift.php index 1c56d180e2..11186a9f39 100644 --- a/apps/files_external/lib/swift.php +++ b/apps/files_external/lib/swift.php @@ -187,7 +187,7 @@ class Swift extends \OC\Files\Storage\Common { public function rmdir($path) { $path = $this->normalizePath($path); - if (!$this->is_dir($path)) { + if (!$this->is_dir($path) or !$this->isDeletable($path)) { return false; } diff --git a/lib/private/files/storage/local.php b/lib/private/files/storage/local.php index 9df6cdef2a..0a612ae505 100644 --- a/lib/private/files/storage/local.php +++ b/lib/private/files/storage/local.php @@ -39,6 +39,9 @@ if (\OC_Util::runningOnWindows()) { } public function rmdir($path) { + if (!$this->isDeletable($path)) { + return false; + } try { $it = new \RecursiveIteratorIterator( new \RecursiveDirectoryIterator($this->datadir . $path), diff --git a/lib/private/files/storage/mappedlocal.php b/lib/private/files/storage/mappedlocal.php index 0760d842ea..0a21d2938b 100644 --- a/lib/private/files/storage/mappedlocal.php +++ b/lib/private/files/storage/mappedlocal.php @@ -38,6 +38,9 @@ class MappedLocal extends \OC\Files\Storage\Common { } public function rmdir($path) { + if (!$this->isDeletable($path)) { + return false; + } try { $it = new \RecursiveIteratorIterator( new \RecursiveDirectoryIterator($this->buildPath($path)), From 2f22e67570bd7b3e99d0f01b59c0e1808a8b5189 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 25 Aug 2014 14:28:35 +0200 Subject: [PATCH 3/4] Also check if the file itself is updatable --- apps/files_external/lib/streamwrapper.php | 2 +- apps/files_external/lib/swift.php | 2 +- lib/private/files/storage/common.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/files_external/lib/streamwrapper.php b/apps/files_external/lib/streamwrapper.php index adccb2919e..b55bcf94af 100644 --- a/apps/files_external/lib/streamwrapper.php +++ b/apps/files_external/lib/streamwrapper.php @@ -21,7 +21,7 @@ abstract class StreamWrapper extends Common { } public function rmdir($path) { - if ($this->file_exists($path) and $this->isDeletable($path)) { + if ($this->file_exists($path) && $this->isDeletable($path)) { $dh = $this->opendir($path); while (($file = readdir($dh)) !== false) { if ($this->is_dir($path . '/' . $file)) { diff --git a/apps/files_external/lib/swift.php b/apps/files_external/lib/swift.php index 11186a9f39..6a1e12986f 100644 --- a/apps/files_external/lib/swift.php +++ b/apps/files_external/lib/swift.php @@ -187,7 +187,7 @@ class Swift extends \OC\Files\Storage\Common { public function rmdir($path) { $path = $this->normalizePath($path); - if (!$this->is_dir($path) or !$this->isDeletable($path)) { + if (!$this->is_dir($path) || !$this->isDeletable($path)) { return false; } diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index 4680971b27..483a9ac666 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -95,11 +95,11 @@ abstract class Common implements \OC\Files\Storage\Storage { } public function isDeletable($path) { - if ($path === '' or $path === '/') { + if ($path === '' || $path === '/') { return false; } $parent = dirname($path); - return $this->isUpdatable($parent); + return $this->isUpdatable($parent) and $this->isUpdatable($path); } public function isSharable($path) { From 33c0d2f743c82facc7847b62497913952baef296 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 17 Sep 2014 11:36:08 +0200 Subject: [PATCH 4/4] Fix mapping of relative paths --- lib/private/files/mapper.php | 44 +++++++++++++++++++--------- lib/private/files/storage/common.php | 2 +- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/lib/private/files/mapper.php b/lib/private/files/mapper.php index 94dda807c2..5e78ef03dd 100644 --- a/lib/private/files/mapper.php +++ b/lib/private/files/mapper.php @@ -66,8 +66,8 @@ class Mapper */ public function copy($path1, $path2) { - $path1 = $this->stripLast($path1); - $path2 = $this->stripLast($path2); + $path1 = $this->resolveRelativePath($path1); + $path2 = $this->resolveRelativePath($path2); $physicPath1 = $this->logicToPhysical($path1, true); $physicPath2 = $this->logicToPhysical($path2, true); @@ -113,18 +113,11 @@ class Mapper return ''; } - private function stripLast($path) { - if (substr($path, -1) == '/') { - $path = substr_replace($path, '', -1); - } - return $path; - } - /** * @param string $logicPath */ private function resolveLogicPath($logicPath) { - $logicPath = $this->stripLast($logicPath); + $logicPath = $this->resolveRelativePath($logicPath); $sql = 'SELECT * FROM `*PREFIX*file_map` WHERE `logic_path_hash` = ?'; $result = \OC_DB::executeAudited($sql, array(md5($logicPath))); $result = $result->fetchRow(); @@ -136,7 +129,7 @@ class Mapper } private function resolvePhysicalPath($physicalPath) { - $physicalPath = $this->stripLast($physicalPath); + $physicalPath = $this->resolveRelativePath($physicalPath); $sql = \OC_DB::prepare('SELECT * FROM `*PREFIX*file_map` WHERE `physic_path_hash` = ?'); $result = \OC_DB::executeAudited($sql, array(md5($physicalPath))); $result = $result->fetchRow(); @@ -144,12 +137,35 @@ class Mapper return $result['logic_path']; } + private function resolveRelativePath($path) { + $explodedPath = explode('/', $path); + $pathArray = array(); + foreach ($explodedPath as $pathElement) { + if (empty($pathElement) || ($pathElement == '.')) { + continue; + } elseif ($pathElement == '..') { + if (count($pathArray) == 0) { + return false; + } + array_pop($pathArray); + } else { + array_push($pathArray, $pathElement); + } + } + if (substr($path, 0, 1) == '/') { + $path = '/'; + } else { + $path = ''; + } + return $path.implode('/', $pathArray); + } + /** * @param string $logicPath * @param boolean $store */ private function create($logicPath, $store) { - $logicPath = $this->stripLast($logicPath); + $logicPath = $this->resolveRelativePath($logicPath); $index = 0; // create the slugified path @@ -205,8 +221,8 @@ class Mapper } } - $sluggedPath = $this->unchangedPhysicalRoot . implode('/', $sluggedElements); - return $this->stripLast($sluggedPath); + $sluggedPath = $this->unchangedPhysicalRoot.implode('/', $sluggedElements); + return $this->resolveRelativePath($sluggedPath); } /** diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index 483a9ac666..975f44df54 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -99,7 +99,7 @@ abstract class Common implements \OC\Files\Storage\Storage { return false; } $parent = dirname($path); - return $this->isUpdatable($parent) and $this->isUpdatable($path); + return $this->isUpdatable($parent) && $this->isUpdatable($path); } public function isSharable($path) {