From bee918693a70427efc2f33034176e89d7a776bc3 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 30 May 2016 15:44:19 +0200 Subject: [PATCH] dissalow symlinks in local storages that point outside the datadir --- lib/private/Files/Storage/Local.php | 30 +++++++++++++++++++++++++- lib/private/legacy/helper.php | 4 +++- tests/lib/Files/Storage/LocalTest.php | 31 +++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/Storage/Local.php b/lib/private/Files/Storage/Local.php index acd4c3b483..005b5f9ab9 100644 --- a/lib/private/Files/Storage/Local.php +++ b/lib/private/Files/Storage/Local.php @@ -33,20 +33,31 @@ */ namespace OC\Files\Storage; + +use OCP\Files\ForbiddenException; + /** * for local filestore, we only have to map the paths */ class Local extends \OC\Files\Storage\Common { protected $datadir; + protected $dataDirLength; + + protected $allowSymlinks = false; + + protected $realDataDir; + public function __construct($arguments) { if (!isset($arguments['datadir']) || !is_string($arguments['datadir'])) { throw new \InvalidArgumentException('No data directory set for local storage'); } $this->datadir = $arguments['datadir']; + $this->realDataDir = rtrim(realpath($this->datadir), '/') . '/'; if (substr($this->datadir, -1) !== '/') { $this->datadir .= '/'; } + $this->dataDirLength = strlen($this->realDataDir); } public function __destruct() { @@ -337,10 +348,27 @@ class Local extends \OC\Files\Storage\Common { * * @param string $path * @return string + * @throws ForbiddenException */ public function getSourcePath($path) { $fullPath = $this->datadir . $path; - return $fullPath; + if ($this->allowSymlinks || $path === '') { + return $fullPath; + } + $pathToResolve = $fullPath; + $realPath = realpath($pathToResolve); + while ($realPath === false) { // for non existing files check the parent directory + $pathToResolve = dirname($pathToResolve); + $realPath = realpath($pathToResolve); + } + if ($realPath) { + $realPath = $realPath . '/'; + } + if (substr($realPath, 0, $this->dataDirLength) === $this->realDataDir) { + return $fullPath; + } else { + throw new ForbiddenException("Following symlinks is not allowed ('$fullPath' -> '$realPath' not inside '{$this->realDataDir}')", false); + } } /** diff --git a/lib/private/legacy/helper.php b/lib/private/legacy/helper.php index f107d47faf..21fb3cbc5a 100644 --- a/lib/private/legacy/helper.php +++ b/lib/private/legacy/helper.php @@ -206,7 +206,9 @@ class OC_Helper { foreach ($files as $fileInfo) { /** @var SplFileInfo $fileInfo */ - if ($fileInfo->isDir()) { + if ($fileInfo->isLink()) { + unlink($fileInfo->getPathname()); + } else if ($fileInfo->isDir()) { rmdir($fileInfo->getRealPath()); } else { unlink($fileInfo->getRealPath()); diff --git a/tests/lib/Files/Storage/LocalTest.php b/tests/lib/Files/Storage/LocalTest.php index 7b8ae6a24b..cca4d6a667 100644 --- a/tests/lib/Files/Storage/LocalTest.php +++ b/tests/lib/Files/Storage/LocalTest.php @@ -84,5 +84,36 @@ class LocalTest extends Storage { public function testInvalidArgumentsNoArray() { new \OC\Files\Storage\Local(null); } + + /** + * @expectedException \OCP\Files\ForbiddenException + */ + public function testDisallowSymlinksOutsideDatadir() { + $subDir1 = $this->tmpDir . 'sub1'; + $subDir2 = $this->tmpDir . 'sub2'; + $sym = $this->tmpDir . 'sub1/sym'; + mkdir($subDir1); + mkdir($subDir2); + + symlink($subDir2, $sym); + + $storage = new \OC\Files\Storage\Local(['datadir' => $subDir1]); + + $storage->file_put_contents('sym/foo', 'bar'); + } + + public function testDisallowSymlinksInsideDatadir() { + $subDir1 = $this->tmpDir . 'sub1'; + $subDir2 = $this->tmpDir . 'sub1/sub2'; + $sym = $this->tmpDir . 'sub1/sym'; + mkdir($subDir1); + mkdir($subDir2); + + symlink($subDir2, $sym); + + $storage = new \OC\Files\Storage\Local(['datadir' => $subDir1]); + + $storage->file_put_contents('sym/foo', 'bar'); + } }