Sanitize input and small fixes

Signed-off-by: Julius Haertl <jus@bitgrid.net>

apps/theming/appinfo/routes.php

@@ -64,13 +64,13 @@ return ['routes' => [
 		'name'	=> 'Icon#getFavicon',
 		'url' => '/favicon/{app}',
 		'verb' => 'GET',
-		'defaults' => array("app" => "core"),
+		'defaults' => array('app' => 'core'),
 	],
 	[
 		'name'	=> 'Icon#getTouchIcon',
 		'url' => '/icon/{app}',
 		'verb' => 'GET',
-		'defaults' => array("app" => "core"),
+		'defaults' => array('app' => 'core'),
 	],
 	[
 		'name'	=> 'Icon#getThemedIcon',

apps/theming/lib/Controller/IconController.php

@@ -27,6 +27,7 @@ use OCA\Theming\ImageManager;
 use OCA\Theming\ThemingDefaults;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\AppFramework\Http\FileDisplayResponse;
 use OCP\AppFramework\Utility\ITimeFactory;
@@ -131,7 +132,8 @@ class IconController extends Controller {
 			$response->addHeader('Expires', $expires->format(\DateTime::RFC2822));
 			$response->addHeader('Pragma', 'cache');
 		} else {
-			$response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND);
+			$response = new Response();
+			$response->setStatus(Http::STATUS_NOT_FOUND);
 			$response->cacheFor(0);
 			$response->setLastModified(new \DateTime('now', new \DateTimeZone('GMT')));
 		}
@@ -163,7 +165,8 @@ class IconController extends Controller {
 			$response->addHeader('Expires', $expires->format(\DateTime::RFC2822));
 			$response->addHeader('Pragma', 'cache');
 		} else {
-			$response = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND);
+			$response = new Response();
+			$response->setStatus(Http::STATUS_NOT_FOUND);
 			$response->cacheFor(0);
 			$response->setLastModified(new \DateTime('now', new \DateTimeZone('GMT')));
 		}

apps/theming/lib/ImageManager.php

@@ -109,7 +109,4 @@ class ImageManager {
 			}
 		}
 	}
-
-
-
 }

apps/theming/lib/ThemingDefaults.php

@@ -58,6 +58,7 @@ class ThemingDefaults extends \OC_Defaults {
 	 * @param IURLGenerator $urlGenerator
 	 * @param \OC_Defaults $defaults
 	 * @param IRootFolder $rootFolder
+	 * @param ICacheFactory $cacheFactory
 	 */
 	public function __construct(IConfig $config,
 								IL10N $l,

apps/theming/lib/Util.php

@@ -28,9 +28,18 @@ use OCP\Files\IRootFolder;
 
 class Util {
 
+	/** @var IConfig */
 	private $config;
+
+	/** @var IRootFolder */
 	private $rootFolder;
 
+	/**
+	 * Util constructor.
+	 *
+	 * @param IConfig $config
+	 * @param IRootFolder $rootFolder
+	 */
 	public function __construct(IConfig $config, IRootFolder $rootFolder) {
 		$this->config = $config;
 		$this->rootFolder = $rootFolder;
@@ -98,7 +107,9 @@ class Util {
 	 * @return string path to app icon / logo
 	 */
 	public function getAppIcon($app) {
+		$app = str_replace(array('\0', '/', '\\', '..'), '', $app);
 		$appPath = \OC_App::getAppPath($app);
+		if ($appPath !== false) {
 			$icon = $appPath . '/img/' . $app . '.svg';
 			if (file_exists($icon)) {
 				return $icon;
@@ -107,6 +118,7 @@ class Util {
 			if (file_exists($icon)) {
 				return $icon;
 			}
+		}
 		if($this->config->getAppValue('theming', 'logoMime', '') !== '' && $this->rootFolder->nodeExists('/themedinstancelogo')) {
 			return $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data/') . '/themedinstancelogo';
 		}
@@ -119,6 +131,8 @@ class Util {
 	 * @return string absolute path to image
 	 */
 	public function getAppImage($app, $image) {
+		$app = str_replace(array('\0', '/', '\\', '..'), '', $app);
+		$image = str_replace(array('\0', '\\', '..'), '', $image);
 		$appPath = \OC_App::getAppPath($app);
 			if ($app === "core") {
 				$icon = \OC::$SERVERROOT . '/core/img/' . $image;
@@ -126,6 +140,7 @@ class Util {
 					return $icon;
 				}
 			}
+		if ($appPath !== false) {
 			$icon = $appPath . '/img/' . $image;
 			if (file_exists($icon)) {
 				return $icon;
@@ -146,6 +161,7 @@ class Util {
 			if (file_exists($icon)) {
 				return $icon;
 			}
+		}
 		return false;
 	}
 

apps/theming/tests/Controller/IconControllerTest.php

@@ -30,7 +30,6 @@ use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\Files\IRootFolder;
 use OCP\Files\NotFoundException;
 use OCP\IConfig;
-use OCP\IL10N;
 use OCP\IRequest;
 use Test\TestCase;
 use OCA\Theming\Util;
@@ -47,7 +46,7 @@ class IconControllerTest extends TestCase {
 	private $util;
 	/** @var \OCP\AppFramework\Utility\ITimeFactory */
 	private $timeFactory;
-	/** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IconController|\PHPUnit_Framework_MockObject_MockObject */
 	private $iconController;
 	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
 	private $config;
@@ -110,8 +109,6 @@ class IconControllerTest extends TestCase {
 		$expected->addHeader('Expires', $expires->format(\DateTime::RFC2822));
 		$expected->addHeader('Pragma', 'cache');
 		@$this->assertEquals($expected, $this->iconController->getThemedIcon('core', 'filetypes/folder.svg'));
-
-
 	}
 
 	public function testGetFaviconDefault() {
@@ -152,7 +149,8 @@ class IconControllerTest extends TestCase {
 		$this->themingDefaults->expects($this->any())
 			->method('shouldReplaceIcons')
 			->willReturn(false);
-		$expected = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND);
+		$expected = new Http\Response();
+		$expected->setStatus(Http::STATUS_NOT_FOUND);
 		$expected->cacheFor(0);
 		$expected->setLastModified(new \DateTime('now', new \DateTimeZone('GMT')));
 		$this->assertEquals($expected, $this->iconController->getFavicon());
@@ -196,7 +194,8 @@ class IconControllerTest extends TestCase {
 		$this->themingDefaults->expects($this->any())
 			->method('shouldReplaceIcons')
 			->willReturn(false);
-		$expected = new DataDisplayResponse(null, Http::STATUS_NOT_FOUND);
+		$expected = new Http\Response();
+		$expected->setStatus(Http::STATUS_NOT_FOUND);
 		$expected->cacheFor(0);
 		$expected->setLastModified(new \DateTime('now', new \DateTimeZone('GMT')));
 		$this->assertEquals($expected, $this->iconController->getTouchIcon());

apps/theming/tests/UtilTest.php

@@ -97,14 +97,13 @@ class UtilTest extends TestCase {
 		$expected = 'PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIGhlaWdodD0iMTYiIHdpZHRoPSIxNiI+PHBhdGggZD0iTTggMWE3IDcgMCAwIDAtNyA3IDcgNyAwIDAgMCA3IDcgNyA3IDAgMCAwIDctNyA3IDcgMCAwIDAtNy03em0wIDFhNiA2IDAgMCAxIDYgNiA2IDYgMCAwIDEtNiA2IDYgNiAwIDAgMS02LTYgNiA2IDAgMCAxIDYtNnptMCAyYTQgNCAwIDEgMCAwIDggNCA0IDAgMCAwIDAtOHoiIGZpbGw9IiNmZmZmZmYiLz48L3N2Zz4=';
 		$this->assertEquals($expected, $button);
 	}
+
 	public function testGenerateRadioButtonBlack() {
 		$button = $this->util->generateRadioButton('#000000');
 		$expected = 'PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIGhlaWdodD0iMTYiIHdpZHRoPSIxNiI+PHBhdGggZD0iTTggMWE3IDcgMCAwIDAtNyA3IDcgNyAwIDAgMCA3IDcgNyA3IDAgMCAwIDctNyA3IDcgMCAwIDAtNy03em0wIDFhNiA2IDAgMCAxIDYgNiA2IDYgMCAwIDEtNiA2IDYgNiAwIDAgMS02LTYgNiA2IDAgMCAxIDYtNnptMCAyYTQgNCAwIDEgMCAwIDggNCA0IDAgMCAwIDAtOHoiIGZpbGw9IiMwMDAwMDAiLz48L3N2Zz4=';
 		$this->assertEquals($expected, $button);
 	}
 
-
-
 	/**
 	 * @dataProvider dataGetAppIcon
 	 */
@@ -137,6 +136,7 @@ class UtilTest extends TestCase {
 	public function testGetAppImage($app, $image, $expected) {
 		$this->assertEquals($expected, $this->util->getAppImage($app, $image));
 	}
+
 	public function dataGetAppImage() {
 		return [
 			['core', 'logo.svg', \OC::$SERVERROOT . '/core/img/logo.svg'],