Merge pull request #36104 from nextcloud/backport/36095/stable24

[stable24] Limit key names when uploading theme images

apps/theming/lib/Controller/ThemingController.php

@@ -63,6 +63,7 @@ use OCP\IURLGenerator;
  * @package OCA\Theming\Controller
  */
 class ThemingController extends Controller {
+	const VALID_UPLOAD_KEYS = ['logo', 'logoheader', 'background', 'favicon'];
 	/** @var ThemingDefaults */
 	private $themingDefaults;
 	/** @var IL10N */
@@ -215,6 +216,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 = [

apps/theming/tests/Controller/ThemingControllerTest.php

@@ -249,6 +249,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.