Merge pull request #15918 from owncloud/harden-avatar

Filter potential dangerous filenames for avatars

lib/private/avatar.php

@@ -8,6 +8,7 @@
  * @author Robin McCorkell <rmccorkell@karoshi.org.uk>
  * @author Roeland Jago Douma <roeland@famdouma.nl>
  * @author Thomas Müller <thomas.mueller@tmit.eu>
+ * @author Lukas Reschke <lukas@owncloud.com>
  *
  * @copyright Copyright (c) 2015, ownCloud, Inc.
  * @license AGPL-3.0
@@ -26,23 +27,28 @@
  *
  */
 
- namespace OC;
+namespace OC;
 
- use OC_Image;
+use OC\Files\Filesystem;
+use OC_Image;
 
 /**
  * This class gets and sets users avatars.
  */
 
 class Avatar implements \OCP\IAvatar {
-
+	/** @var Files\View  */
 	private $view;
 
 	/**
 	 * constructor
 	 * @param string $user user to do avatar-management with
-	*/
+	 * @throws \Exception In case the username is potentially dangerous
+	 */
 	public function __construct ($user) {
+		if(!Filesystem::isValidPath($user)) {
+			throw new \Exception('Username may not contain slashes');
+		}
 		$this->view = new \OC\Files\View('/'.$user);
 	}
 

lib/private/avatarmanager.php

@@ -37,6 +37,7 @@ class AvatarManager implements IAvatarManager {
 	 * @see \OCP\IAvatar
 	 * @param string $user the ownCloud user id
 	 * @return \OCP\IAvatar
+	 * @throws \Exception In case the username is potentially dangerous
 	 */
 	public function getAvatar($user) {
 		return new Avatar($user);

tests/lib/avatar.php

@@ -34,6 +34,29 @@ class Test_Avatar extends \Test\TestCase {
 		}
 	}
 
+	/**
+	 * @return array
+	 */
+	public function traversalProvider() {
+		return [
+			['Pot\..\entiallyDangerousUsername'],
+			['Pot/..\entiallyDangerousUsername'],
+			['PotentiallyDangerousUsername/..'],
+			['PotentiallyDangerousUsername\../'],
+			['/../PotentiallyDangerousUsername'],
+		];
+	}
+
+	/**
+	 * @dataProvider traversalProvider
+	 * @expectedException \Exception
+	 * @expectedExceptionMessage Username may not contain slashes
+	 * @param string $dangerousUsername
+	 */
+	public function testAvatarTraversal($dangerousUsername) {
+		new Avatar($dangerousUsername);
+	}
+
 	public function testAvatar() {
 
 		$avatar = new Avatar($this->user);