check for filename blacklist in OC_Filesystem::isValidPath

Author: icewind1991

lib/filesystem.php

@@ -368,6 +368,9 @@ static public function isValidPath($path){
 		if(strstr($path,'/../') || strrchr($path, '/') === '/..' ){
 			return false;
 		}
+		if(self::isFileBlacklisted($path)){
+			return false;
+		}
 		return true;
 	}
 
@@ -376,21 +379,23 @@ static public function isValidPath($path){
 	 * Listens to write and rename hooks
 	 * @param array $data from hook
 	 */
-	static public function isBlacklisted($data){
-		$blacklist = array('.htaccess');
+	static public function isBlacklisted($data) {
 		if (isset($data['path'])) {
 			$path = $data['path'];
 		} else if (isset($data['newpath'])) {
 			$path = $data['newpath'];
 		}
 		if (isset($path)) {
-			$filename = strtolower(basename($path));
-			if (in_array($filename, $blacklist)) {
-				$data['run'] = false;
-			}
+			$data['run'] = !self::isFileBlacklisted($path);
 		}
 	}
-	
+
+	static public function isFileBlacklisted($path){
+		$blacklist = array('.htaccess');
+		$filename = strtolower(basename($path));
+		return in_array($filename, $blacklist);
+	}
+
 	/**
 	 * following functions are equivalent to their php builtin equivalents for arguments/return values.
 	 */

tests/lib/filesystem.php

@@ -45,20 +45,98 @@ public function setUp(){
 		OC_Filesystem::clearMounts();
 	}
 
-	public function testMount(){
-		OC_Filesystem::mount('OC_Filestorage_Local',self::getStorageData(),'/');
-		$this->assertEqual('/',OC_Filesystem::getMountPoint('/'));
-		$this->assertEqual('/',OC_Filesystem::getMountPoint('/some/folder'));
-		$this->assertEqual('',OC_Filesystem::getInternalPath('/'));
-		$this->assertEqual('some/folder',OC_Filesystem::getInternalPath('/some/folder'));
-
-		OC_Filesystem::mount('OC_Filestorage_Local',self::getStorageData(),'/some');
-		$this->assertEqual('/',OC_Filesystem::getMountPoint('/'));
-		$this->assertEqual('/some/',OC_Filesystem::getMountPoint('/some/folder'));
-		$this->assertEqual('/some/',OC_Filesystem::getMountPoint('/some/'));
-		$this->assertEqual('/',OC_Filesystem::getMountPoint('/some'));
-		$this->assertEqual('folder',OC_Filesystem::getInternalPath('/some/folder'));
+	public function testMount() {
+		OC_Filesystem::mount('OC_Filestorage_Local', self::getStorageData(), '/');
+		$this->assertEqual('/', OC_Filesystem::getMountPoint('/'));
+		$this->assertEqual('/', OC_Filesystem::getMountPoint('/some/folder'));
+		$this->assertEqual('', OC_Filesystem::getInternalPath('/'));
+		$this->assertEqual('some/folder', OC_Filesystem::getInternalPath('/some/folder'));
+
+		OC_Filesystem::mount('OC_Filestorage_Local', self::getStorageData(), '/some');
+		$this->assertEqual('/', OC_Filesystem::getMountPoint('/'));
+		$this->assertEqual('/some/', OC_Filesystem::getMountPoint('/some/folder'));
+		$this->assertEqual('/some/', OC_Filesystem::getMountPoint('/some/'));
+		$this->assertEqual('/', OC_Filesystem::getMountPoint('/some'));
+		$this->assertEqual('folder', OC_Filesystem::getInternalPath('/some/folder'));
+	}
+
+	public function testNormalize() {
+		$this->assertEqual('/path', OC_Filesystem::normalizePath('/path/'));
+		$this->assertEqual('/path/', OC_Filesystem::normalizePath('/path/', false));
+		$this->assertEqual('/path', OC_Filesystem::normalizePath('path'));
+		$this->assertEqual('/path', OC_Filesystem::normalizePath('\path'));
+		$this->assertEqual('/foo/bar', OC_Filesystem::normalizePath('/foo//bar/'));
+		$this->assertEqual('/foo/bar', OC_Filesystem::normalizePath('/foo////bar'));
+		if (class_exists('Normalizer')) {
+			$this->assertEqual("/foo/bar\xC3\xBC", OC_Filesystem::normalizePath("/foo/baru\xCC\x88"));
+		}
+	}
+
+	public function testBlacklist() {
+		OC_Hook::clear('OC_Filesystem');
+		OC_Hook::connect('OC_Filesystem', 'write', 'OC_Filesystem', 'isBlacklisted');
+		OC_Hook::connect('OC_Filesystem', 'rename', 'OC_Filesystem', 'isBlacklisted');
+
+		$run = true;
+		OC_Hook::emit(
+			OC_Filesystem::CLASSNAME,
+			OC_Filesystem::signal_write,
+			array(
+				OC_Filesystem::signal_param_path => '/test/.htaccess',
+				OC_Filesystem::signal_param_run => &$run
+			)
+		);
+		$this->assertFalse($run);
+
+		if (OC_Filesystem::getView()) {
+			$user = OC_User::getUser();
+		} else {
+			$user = uniqid();
+			OC_Filesystem::init('/' . $user . '/files');
+		}
+
+		OC_Filesystem::mount('OC_Filestorage_Temporary', array(), '/');
+
+		$rootView = new OC_FilesystemView('');
+		$rootView->mkdir('/' . $user);
+		$rootView->mkdir('/' . $user . '/files');
+
+		$this->assertFalse($rootView->file_put_contents('/.htaccess', 'foo'));
+		$this->assertFalse(OC_Filesystem::file_put_contents('/.htaccess', 'foo'));
+		$fh = fopen(__FILE__, 'r');
+		$this->assertFalse(OC_Filesystem::file_put_contents('/.htaccess', $fh));
+	}
+
+	public function testHooks() {
+		if (OC_Filesystem::getView()) {
+			$user = OC_User::getUser();
+		} else {
+			$user = uniqid();
+			OC_Filesystem::init('/' . $user . '/files');
+		}
+		OC_Hook::clear('OC_Filesystem');
+		OC_Hook::connect('OC_Filesystem', 'post_write', $this, 'dummyHook');
+
+		OC_Filesystem::mount('OC_Filestorage_Temporary', array(), '/');
+
+		$rootView = new OC_FilesystemView('');
+		$rootView->mkdir('/' . $user);
+		$rootView->mkdir('/' . $user . '/files');
+
+		OC_Filesystem::file_put_contents('/foo', 'foo');
+		OC_Filesystem::mkdir('/bar');
+		OC_Filesystem::file_put_contents('/bar//foo', 'foo');
+
+		$tmpFile = OC_Helper::tmpFile();
+		file_put_contents($tmpFile, 'foo');
+		$fh = fopen($tmpFile, 'r');
+		OC_Filesystem::file_put_contents('/bar//foo', $fh);
+	}
+
+	public function dummyHook($arguments) {
+		$path = $arguments['path'];
+		$this->assertEqual($path, OC_Filesystem::normalizePath($path)); //the path passed to the hook should already be normalized
 	}
 }
 
-?>
+?>