From e90f9e37b3d5ad98befd98fe5f1de4731775d94d Mon Sep 17 00:00:00 2001
From: Joas Schilling <coding@schilljs.com>
Date: Mon, 7 Oct 2024 22:10:09 +0200
Subject: [PATCH] fix(appmanager): Fix tainted file path when loading appinfos

Signed-off-by: Joas Schilling <coding@schilljs.com>
---
 build/psalm-baseline-security.xml |  5 -----
 build/psalm-baseline.xml          |  4 ----
 lib/private/App/AppManager.php    | 35 ++++++++++++++++++-------------
 lib/private/Installer.php         |  2 +-
 lib/private/legacy/OC_App.php     |  5 +++--
 lib/public/App/IAppManager.php    | 10 ++++++++-
 6 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/build/psalm-baseline-security.xml b/build/psalm-baseline-security.xml
index c42b10d75c677..32939a14aec01 100644
--- a/build/psalm-baseline-security.xml
+++ b/build/psalm-baseline-security.xml
@@ -22,11 +22,6 @@
       <code><![CDATA['Location: ' . \OC::$WEBROOT . '/']]></code>
     </TaintedHeader>
   </file>
-  <file src="lib/private/App/InfoParser.php">
-    <TaintedFile>
-      <code><![CDATA[$file]]></code>
-    </TaintedFile>
-  </file>
   <file src="lib/private/AppFramework/Utility/SimpleContainer.php">
     <TaintedCallable>
       <code><![CDATA[$name]]></code>
diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
index 16fe161a9ba4d..2c2784e7fb120 100644
--- a/build/psalm-baseline.xml
+++ b/build/psalm-baseline.xml
@@ -2750,10 +2750,6 @@
     <NullArgument>
       <code><![CDATA[null]]></code>
     </NullArgument>
-    <TypeDoesNotContainNull>
-      <code><![CDATA[$appId === null]]></code>
-      <code><![CDATA[$appId === null]]></code>
-    </TypeDoesNotContainNull>
   </file>
   <file src="lib/private/legacy/OC_Helper.php">
     <InvalidArrayOffset>
diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php
index 4ffddef98c324..ff923363ea874 100644
--- a/lib/private/App/AppManager.php
+++ b/lib/private/App/AppManager.php
@@ -744,28 +744,35 @@ public function getAppsNeedingUpgrade($version) {
 	 */
 	public function getAppInfo(string $appId, bool $path = false, $lang = null) {
 		if ($path) {
-			$file = $appId;
-		} else {
-			if ($lang === null && isset($this->appInfos[$appId])) {
-				return $this->appInfos[$appId];
-			}
-			try {
-				$appPath = $this->getAppPath($appId);
-			} catch (AppPathNotFoundException $e) {
-				return null;
-			}
-			$file = $appPath . '/appinfo/info.xml';
+			throw new \InvalidArgumentException('Calling IAppManager::getAppInfo() with a path is no longer supported. Please call IAppManager::getAppInfoByPath() instead and verify that the path is good before calling.');
+		}
+		if ($lang === null && isset($this->appInfos[$appId])) {
+			return $this->appInfos[$appId];
+		}
+		try {
+			$appPath = $this->getAppPath($appId);
+		} catch (AppPathNotFoundException $e) {
+			return null;
+		}
+		$file = $appPath . '/appinfo/info.xml';
+
+		return $this->getAppInfoByPath($file, $lang);
+	}
+
+	public function getAppInfoByPath(string $path, ?string $lang = null): ?array {
+		if (!str_ends_with($path, '/appinfo/info.xml')) {
+			return null;
 		}
 
 		$parser = new InfoParser($this->memCacheFactory->createLocal('core.appinfo'));
-		$data = $parser->parse($file);
+		$data = $parser->parse($path);
 
 		if (is_array($data)) {
 			$data = \OC_App::parseAppInfo($data, $lang);
 		}
 
-		if ($lang === null) {
-			$this->appInfos[$appId] = $data;
+		if ($lang === null && isset($data['id'])) {
+			$this->appInfos[$data['id']] = $data;
 		}
 
 		return $data;
diff --git a/lib/private/Installer.php b/lib/private/Installer.php
index d5500c07a3cf9..00fdd84c1bc83 100644
--- a/lib/private/Installer.php
+++ b/lib/private/Installer.php
@@ -65,7 +65,7 @@ public function installApp(string $appId, bool $forceEnable = false): string {
 		}
 
 		$l = \OCP\Util::getL10N('core');
-		$info = \OCP\Server::get(IAppManager::class)->getAppInfo($basedir . '/appinfo/info.xml', true, $l->getLanguageCode());
+		$info = \OCP\Server::get(IAppManager::class)->getAppInfoByPath($basedir . '/appinfo/info.xml', $l->getLanguageCode());
 
 		if (!is_array($info)) {
 			throw new \Exception(
diff --git a/lib/private/legacy/OC_App.php b/lib/private/legacy/OC_App.php
index a9f8b24d8317f..6afd4086cb3eb 100644
--- a/lib/private/legacy/OC_App.php
+++ b/lib/private/legacy/OC_App.php
@@ -313,7 +313,8 @@ public static function findAppInDirectories(string $appId, bool $ignoreCache = f
 	 * @deprecated 11.0.0 use \OCP\Server::get(IAppManager)->getAppPath()
 	 */
 	public static function getAppPath(string $appId, bool $refreshAppPath = false) {
-		if ($appId === null || trim($appId) === '') {
+		$appId = self::cleanAppId($appId);
+		if ($appId === '') {
 			return false;
 		}
 
@@ -346,7 +347,7 @@ public static function getAppWebPath(string $appId) {
 	 */
 	public static function getAppVersionByPath(string $path): string {
 		$infoFile = $path . '/appinfo/info.xml';
-		$appData = \OC::$server->getAppManager()->getAppInfo($infoFile, true);
+		$appData = \OCP\Server::get(IAppManager::class)->getAppInfoByPath($infoFile);
 		return $appData['version'] ?? '';
 	}
 
diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php
index 1182f611b299f..0af7cdfc49530 100644
--- a/lib/public/App/IAppManager.php
+++ b/lib/public/App/IAppManager.php
@@ -25,14 +25,22 @@ interface IAppManager {
 	public const BACKEND_CALDAV = 'caldav';
 
 	/**
-	 * Returns the app information from "appinfo/info.xml".
+	 * Returns the app information from "appinfo/info.xml" for an app
 	 *
 	 * @param string|null $lang
 	 * @return array|null
 	 * @since 14.0.0
+	 * @since 31.0.0 Usage of $path is discontinued and throws an \InvalidArgumentException, use {@see self::getAppInfoByPath} instead.
 	 */
 	public function getAppInfo(string $appId, bool $path = false, $lang = null);
 
+	/**
+	 * Returns the app information from a given path ending with "/appinfo/info.xml"
+	 *
+	 * @since 31.0.0
+	 */
+	public function getAppInfoByPath(string $path, ?string $lang = null): ?array;
+
 	/**
 	 * Returns the app information from "appinfo/info.xml".
 	 *