Merge pull request #8156 from nextcloud/stable13-8078

[Stable13] Scss hardening

apps/theming/css/theming.scss

@@ -91,18 +91,20 @@
 }
 
 /* override styles for login screen in guest.css */
-#header .logo {
+@if variable_exists('theming-logo-mime') {
-	background-image: url(#{$image-logo});
+	#header .logo {
-	@if $theming-logo-mime != '' {
+		background-image: url(#{$image-logo});
 		background-size: contain;
 	}
 }
 
-#body-login,
+@if variable_exists('theming-background-mime') {
-#firstrunwizard .firstrunwizard-header,
+	#body-login,
-#theming-preview {
+	#firstrunwizard .firstrunwizard-header,
-	background-image: url(#{$image-login-background});
+	#theming-preview {
-	background-color: $color-primary;
+		background-image: url(#{$image-login-background});
+		background-color: $color-primary;
+	}
 }
 
 input.primary,

apps/theming/lib/ThemingDefaults.php

@@ -242,8 +242,8 @@ class ThemingDefaults extends \OC_Defaults {
 			'theming-background-mime' => "'" . $this->config->getAppValue('theming', 'backgroundMime', '') . "'"
 		];
 
-		$variables['image-logo'] = "'".$this->urlGenerator->getAbsoluteURL($this->getLogo())."'";
+		$variables['image-logo'] = "'".$this->getLogo()."'";
-		$variables['image-login-background'] = "'".$this->urlGenerator->getAbsoluteURL($this->getBackground())."'";
+		$variables['image-login-background'] = "'".$this->getBackground()."'";
 		$variables['image-login-plain'] = 'false';
 
 		if ($this->config->getAppValue('theming', 'color', null) !== null) {

apps/theming/tests/ThemingDefaultsTest.php

@@ -517,18 +517,12 @@ class ThemingDefaultsTest extends TestCase {
 				['theming.Theming.getLoginBackground', [], 'custom-background'],
 			]);
 
-		$this->urlGenerator->expects($this->exactly(2))
-			->method('getAbsoluteURL')
-			->willReturnCallback(function ($path) {
-				return 'absolute-' . $path;
-			});
-
 		$expected = [
 			'theming-cachebuster' => '\'0\'',
 			'theming-logo-mime' => '\'jpeg\'',
 			'theming-background-mime' => '\'jpeg\'',
-			'image-logo' => "'absolute-custom-logo?v=0'",
+			'image-logo' => "'custom-logo?v=0'",
-			'image-login-background' => "'absolute-custom-background?v=0'",
+			'image-login-background' => "'custom-background?v=0'",
 			'color-primary' => $this->defaults->getColorPrimary(),
 			'color-primary-text' => '#ffffff',
 			'image-login-plain' => 'false',

lib/private/Template/SCSSCacher.php

@@ -63,6 +63,9 @@ class SCSSCacher {
 	/** @var ICache */
 	protected $depsCache;
 
+	/** @var null|string */
+	protected $injectedVariables = null;
+
 	/**
 	 * @param ILogger $logger
 	 * @param Factory $appDataFactory
@@ -153,8 +156,9 @@ class SCSSCacher {
 						return false;
 					}
 				}
+				return true;
 			}
-			return true;
+			return false;
 		} catch(NotFoundException $e) {
 			return false;
 		}
@@ -250,6 +254,7 @@ class SCSSCacher {
 	 * We need to regenerate all files when variables change
 	 */
 	private function resetCache() {
+		$this->injectedVariables = null;
 		$appDirectory = $this->appData->getDirectoryListing();
 		if(empty($appDirectory)){
 			return;
@@ -267,10 +272,22 @@ class SCSSCacher {
 	 * @return string SCSS code for variables from OC_Defaults
 	 */
 	private function getInjectedVariables() {
+		if ($this->injectedVariables !== null)
+			return $this->injectedVariables;
 		$variables = '';
 		foreach ($this->defaults->getScssVariables() as $key => $value) {
 			$variables .= '$' . $key . ': ' . $value . ';';
 		}
+
+		// check for valid variables / otherwise fall back to defaults
+		try {
+			$scss = new Compiler();
+			$scss->compile($variables);
+			$this->injectedVariables = $variables;
+		} catch (ParserException $e) {
+			$this->logger->error($e, ['app' => 'core']);
+		}
+
 		return $variables;
 	}
 
@@ -281,7 +298,7 @@ class SCSSCacher {
 	 * @return string
 	 */
 	private function rebaseUrls($css, $webDir) {
-		$re = '/url\([\'"]([\.\w?=\/-]*)[\'"]\)/x';
+		$re = '/url\([\'"]([^\/][\.\w?=\/-]*)[\'"]\)/x';
 		$subst = 'url(\''.$webDir.'/$1\')';
 		return preg_replace($re, $subst, $css);
 	}

tests/lib/Template/SCSSCacherTest.php

@@ -351,11 +351,21 @@ class SCSSCacherTest extends \Test\TestCase {
 		$this->assertFalse($actual);
 	}
 
-	public function testRebaseUrls() {
+	public function dataRebaseUrls() {
+		return [
+			['#id { background-image: url(\'../img/image.jpg\'); }','#id { background-image: url(\'/apps/files/css/../img/image.jpg\'); }'],
+			['#id { background-image: url("../img/image.jpg"); }','#id { background-image: url(\'/apps/files/css/../img/image.jpg\'); }'],
+			['#id { background-image: url(\'/img/image.jpg\'); }','#id { background-image: url(\'/img/image.jpg\'); }'],
+			['#id { background-image: url("http://example.com/test.jpg"); }','#id { background-image: url("http://example.com/test.jpg"); }'],
+		];
+	}
+
+	/**
+	 * @dataProvider dataRebaseUrls
+	 */
+	public function testRebaseUrls($scss, $expected) {
 		$webDir = '/apps/files/css';
-		$css = '#id { background-image: url(\'../img/image.jpg\'); }';
+		$actual = self::invokePrivate($this->scssCacher, 'rebaseUrls', [$scss, $webDir]);
-		$actual = self::invokePrivate($this->scssCacher, 'rebaseUrls', [$css, $webDir]);
-		$expected = '#id { background-image: url(\'/apps/files/css/../img/image.jpg\'); }';
 		$this->assertEquals($expected, $actual);
 	}