Merge pull request #36095 from nextcloud/bugfix/noid/theming-limit-key

Limit key names when uploading theme images

2 files changed, 44 insertions, 1 deletions

diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php
index 8868208197d..a323bac180b 100644
--- a/apps/theming/lib/Controller/ThemingController.php
+++ b/apps/theming/lib/Controller/ThemingController.php
@@ -65,6 +65,8 @@ use ScssPhp\ScssPhp\Compiler;
  * @package OCA\Theming\Controller
  */
 class ThemingController extends Controller {
+	const VALID_UPLOAD_KEYS = ['header', 'logo', 'logoheader', 'background', 'favicon'];
+
 	private ThemingDefaults $themingDefaults;
 	private IL10N $l10n;
 	private IConfig $config;
@@ -191,6 +193,17 @@ class ThemingController extends Controller {
 	 */
 	public function uploadImage(): DataResponse {
 		$key = $this->request->getParam('key');
+		if (!in_array($key, self::VALID_UPLOAD_KEYS, true)) {
+			return new DataResponse(
+				[
+					'data' => [
+						'message' => 'Invalid key'
+					],
+					'status' => 'failure',
+				],
+				Http::STATUS_BAD_REQUEST
+			);
+		}
 		$image = $this->request->getUploadedFile('image');
 		$error = null;
 		$phpFileUploadErrors = [
@@ -354,7 +367,7 @@ class ThemingController extends Controller {
 		// If plain is set, the browser decides of the css priority
 		if ($plain) {
 			$css = ":root { $variables } " . $customCss;
-		} else { 
+		} else {
 			// If not set, we'll rely on the body class
 			$compiler = new Compiler();
 			$compiledCss = $compiler->compileString("[data-theme-$themeId] { $variables $customCss }");
diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php
index 9042a338fb7..4931a148b78 100644
--- a/apps/theming/tests/Controller/ThemingControllerTest.php
+++ b/apps/theming/tests/Controller/ThemingControllerTest.php
@@ -238,6 +238,36 @@ class ThemingControllerTest extends TestCase {
 		$this->assertEquals($expected, $this->themingController->uploadImage());
 	}
 
+	public function testUploadInvalidUploadKey() {
+		$this->request
+			->expects($this->once())
+			->method('getParam')
+			->with('key')
+			->willReturn('invalid');
+		$this->request
+			->expects($this->never())
+			->method('getUploadedFile');
+		$this->l10n
+			->expects($this->any())
+			->method('t')
+			->willReturnCallback(function ($str) {
+				return $str;
+			});
+
+		$expected = new DataResponse(
+			[
+				'data' =>
+					[
+						'message' => 'Invalid key',
+					],
+				'status' => 'failure',
+			],
+			Http::STATUS_BAD_REQUEST
+		);
+
+		$this->assertEquals($expected, $this->themingController->uploadImage());
+	}
+
 	/**
 	 * Checks that trying to upload an SVG favicon without imagemagick
 	 * results in an unsupported media type response.