Check the mimetype before reading the content and catch exception

Signed-off-by: Joas Schilling <coding@schilljs.com>

2 files changed, 62 insertions, 3 deletions

diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php
index b27d1182225..c6f320dd0fe 100644
--- a/core/Controller/AvatarController.php
+++ b/core/Controller/AvatarController.php
@@ -189,7 +189,22 @@ class AvatarController extends Controller {
 					Http::STATUS_BAD_REQUEST
 				);
 			}
-			$content = $node->getContent();
+
+			if ($node->getMimeType() !== 'image/jpeg' && $node->getMimeType() !== 'image/png') {
+				return new JSONResponse(
+					['data' => ['message' => $this->l->t('The selected file is not an image.')]],
+					Http::STATUS_BAD_REQUEST
+				);
+			}
+
+			try {
+				$content = $node->getContent();
+			} catch (\OCP\Files\NotPermittedException $e) {
+				return new JSONResponse(
+					['data' => ['message' => $this->l->t('The selected file cannot be read.')]],
+					Http::STATUS_BAD_REQUEST
+				);
+			}
 		} elseif (!is_null($files)) {
 			if (
 				$files['error'][0] === 0 &&
diff --git a/tests/Core/Controller/AvatarControllerTest.php b/tests/Core/Controller/AvatarControllerTest.php
index af75c4bb751..b87f73366fa 100644
--- a/tests/Core/Controller/AvatarControllerTest.php
+++ b/tests/Core/Controller/AvatarControllerTest.php
@@ -38,6 +38,7 @@ use OCP\Files\Cache\ICache;
 use OCP\Files\File;
 use OCP\Files\IRootFolder;
 use OCP\Files\NotFoundException;
+use OCP\Files\NotPermittedException;
 use OCP\IAvatar;
 use OCP\IAvatarManager;
 use OCP\IL10N;
@@ -334,7 +335,12 @@ class AvatarControllerTest extends \Test\TestCase {
 		//Mock node API call
 		$file = $this->getMockBuilder('OCP\Files\File')
 			->disableOriginalConstructor()->getMock();
-		$file->method('getContent')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg'));
+		$file->expects($this->once())
+			->method('getContent')
+			->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg'));
+		$file->expects($this->once())
+			->method('getMimeType')
+			->willReturn('image/jpeg');
 		$userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock();
 		$this->rootFolder->method('getUserFolder')->with('userid')->willReturn($userFolder);
 		$userFolder->method('get')->willReturn($file);
@@ -365,6 +371,39 @@ class AvatarControllerTest extends \Test\TestCase {
 		$this->assertEquals(['data' => ['message' => 'Please select a file.']], $response->getData());
 	}
 
+	public function testPostAvatarInvalidType() {
+		$file = $this->getMockBuilder('OCP\Files\File')
+			->disableOriginalConstructor()->getMock();
+		$file->expects($this->never())
+			->method('getContent');
+		$file->expects($this->exactly(2))
+			->method('getMimeType')
+			->willReturn('text/plain');
+		$userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock();
+		$this->rootFolder->method('getUserFolder')->with('userid')->willReturn($userFolder);
+		$userFolder->method('get')->willReturn($file);
+
+		$expectedResponse = new Http\JSONResponse(['data' => ['message' => 'The selected file is not an image.']], Http::STATUS_BAD_REQUEST);
+		$this->assertEquals($expectedResponse, $this->avatarController->postAvatar('avatar.jpg'));
+	}
+
+	public function testPostAvatarNotPermittedException() {
+		$file = $this->getMockBuilder('OCP\Files\File')
+			->disableOriginalConstructor()->getMock();
+		$file->expects($this->once())
+			->method('getContent')
+			->willThrowException(new NotPermittedException());
+		$file->expects($this->once())
+			->method('getMimeType')
+			->willReturn('image/jpeg');
+		$userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock();
+		$this->rootFolder->method('getUserFolder')->with('userid')->willReturn($userFolder);
+		$userFolder->method('get')->willReturn($file);
+
+		$expectedResponse = new Http\JSONResponse(['data' => ['message' => 'The selected file cannot be read.']], Http::STATUS_BAD_REQUEST);
+		$this->assertEquals($expectedResponse, $this->avatarController->postAvatar('avatar.jpg'));
+	}
+
 	/**
 	 * Test what happens if the upload of the avatar fails
 	 */
@@ -374,7 +413,12 @@ class AvatarControllerTest extends \Test\TestCase {
 			->will($this->throwException(new \Exception("foo")));
 		$file = $this->getMockBuilder('OCP\Files\File')
 			->disableOriginalConstructor()->getMock();
-		$file->method('getContent')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg'));
+		$file->expects($this->once())
+			->method('getContent')
+			->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg'));
+		$file->expects($this->once())
+			->method('getMimeType')
+			->willReturn('image/jpeg');
 		$userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock();
 		$this->rootFolder->method('getUserFolder')->with('userid')->willReturn($userFolder);
 		$userFolder->method('get')->willReturn($file);