From 78de213b8582f160b9e3acd1d921a6dd1ccd88d9 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Fri, 4 Nov 2016 18:55:00 +0100 Subject: [PATCH] Sanitize input and small fixes Signed-off-by: Julius Haertl --- apps/theming/appinfo/routes.php | 4 +- .../theming/lib/Controller/IconController.php | 7 +- apps/theming/lib/ImageManager.php | 3 - apps/theming/lib/ThemingDefaults.php | 1 + apps/theming/lib/Util.php | 76 +++++++++++-------- .../tests/Controller/IconControllerTest.php | 11 ++- apps/theming/tests/UtilTest.php | 4 +- 7 files changed, 61 insertions(+), 45 deletions(-) diff --git a/apps/theming/appinfo/routes.php b/apps/theming/appinfo/routes.php index a8436ab254f..f4aa2f93162 100644 --- a/apps/theming/appinfo/routes.php +++ b/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', diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 08d5b50120f..519c52f5fa9 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/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'))); } diff --git a/apps/theming/lib/ImageManager.php b/apps/theming/lib/ImageManager.php index e7dcfa92190..4cd43e02054 100644 --- a/apps/theming/lib/ImageManager.php +++ b/apps/theming/lib/ImageManager.php @@ -109,7 +109,4 @@ class ImageManager { } } } - - - } diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index 2c344172127..36f19157637 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/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, diff --git a/apps/theming/lib/Util.php b/apps/theming/lib/Util.php index 84c631092a8..963cf56633b 100644 --- a/apps/theming/lib/Util.php +++ b/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,14 +107,17 @@ class Util { * @return string path to app icon / logo */ public function getAppIcon($app) { + $app = str_replace(array('\0', '/', '\\', '..'), '', $app); $appPath = \OC_App::getAppPath($app); - $icon = $appPath . '/img/' . $app . '.svg'; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/app.svg'; - if(file_exists($icon)) { - return $icon; + if ($appPath !== false) { + $icon = $appPath . '/img/' . $app . '.svg'; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/app.svg'; + 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,32 +131,36 @@ 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; - if(file_exists($icon)) { + if ($app === "core") { + $icon = \OC::$SERVERROOT . '/core/img/' . $image; + if (file_exists($icon)) { + return $icon; + } + } + if ($appPath !== false) { + $icon = $appPath . '/img/' . $image; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.svg'; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.png'; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.gif'; + if (file_exists($icon)) { + return $icon; + } + $icon = $appPath . '/img/' . $image . '.jpg'; + if (file_exists($icon)) { return $icon; } - } - $icon = $appPath . '/img/' . $image; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.svg'; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.png'; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.gif'; - if(file_exists($icon)) { - return $icon; - } - $icon = $appPath . '/img/' . $image . '.jpg'; - if(file_exists($icon)) { - return $icon; } return false; } diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php index 69bbfa0e45e..b1742db018f 100644 --- a/apps/theming/tests/Controller/IconControllerTest.php +++ b/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()); diff --git a/apps/theming/tests/UtilTest.php b/apps/theming/tests/UtilTest.php index 9a53858598c..d82ec8eeb85 100644 --- a/apps/theming/tests/UtilTest.php +++ b/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'], -- 2.39.5