Merge pull request #24899 from owncloud/local-storage-symlinks

dissalow symlinks in local storages that point outside the datadir

6 files changed, 73 insertions, 3 deletions

diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php
index e7226b58196..ddab34605f3 100644
--- a/apps/dav/lib/Connector/Sabre/Directory.php
+++ b/apps/dav/lib/Connector/Sabre/Directory.php
@@ -196,6 +196,8 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node
 				throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage());
 			} catch (\OCP\Files\InvalidPathException $ex) {
 				throw new InvalidPath($ex->getMessage());
+			} catch (ForbiddenException $e) {
+				throw new \Sabre\DAV\Exception\Forbidden();
 			}
 		}
 
diff --git a/apps/dav/lib/Connector/Sabre/ObjectTree.php b/apps/dav/lib/Connector/Sabre/ObjectTree.php
index 599f3fdfd0e..9e7d876187d 100644
--- a/apps/dav/lib/Connector/Sabre/ObjectTree.php
+++ b/apps/dav/lib/Connector/Sabre/ObjectTree.php
@@ -161,6 +161,8 @@ class ObjectTree extends \Sabre\DAV\Tree {
 				throw new \Sabre\DAV\Exception\NotFound('Storage ' . $path . ' is invalid');
 			} catch (LockedException $e) {
 				throw new \Sabre\DAV\Exception\Locked();
+			} catch (ForbiddenException $e) {
+				throw new \Sabre\DAV\Exception\Forbidden();
 			}
 		}
 
diff --git a/lib/private/Files/Cache/Scanner.php b/lib/private/Files/Cache/Scanner.php
index e6bd118d5a5..c17f9bfd51b 100644
--- a/lib/private/Files/Cache/Scanner.php
+++ b/lib/private/Files/Cache/Scanner.php
@@ -38,6 +38,7 @@ use OC\Files\Filesystem;
 use OC\Hooks\BasicEmitter;
 use OCP\Config;
 use OCP\Files\Cache\IScanner;
+use OCP\Files\ForbiddenException;
 use OCP\Files\Storage\ILockingStorage;
 use OCP\Lock\ILockingProvider;
 
@@ -140,7 +141,11 @@ class Scanner extends BasicEmitter implements IScanner {
 				}
 			}
 
-			$data = $this->getData($file);
+			try {
+				$data = $this->getData($file);
+			} catch (ForbiddenException $e) {
+				return null;
+			}
 
 			if ($data) {
 
diff --git a/lib/private/Files/Storage/Local.php b/lib/private/Files/Storage/Local.php
index acd4c3b4838..005b5f9ab91 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 f107d47faf7..21fb3cbc5ab 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 7b8ae6a24b2..cca4d6a6676 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');
+	}
 }