fix(appmanager): Fix tainted file path when loading appinfos

Signed-off-by: Joas Schilling <coding@schilljs.com>

diff --git a/build/psalm-baseline-security.xml b/build/psalm-baseline-security.xml
index c42b10d75c67748589d9d580e1da7f98f07778e5..32939a14aec01732c01512a05a5a9e0d1e7983ac 100644 (file)
--- 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 16fe161a9ba4ddd0138dd48886f0622c74f0a21f..2c2784e7fb120888dfa2479550b5f8eddedff7f6 100644 (file)
--- 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 4ffddef98c32421d89ffadad1c33d8da2dbba4e3..2b6d2a2700bc49c5574e06e08187c01a7d65be17 100644 (file)
--- a/lib/private/App/AppManager.php
+++ b/lib/private/App/AppManager.php
@@ -744,30 +744,39 @@ class AppManager implements IAppManager {
         */
        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) {
+                       return null;
+               }
+               $file = $appPath . '/appinfo/info.xml';
+
+               $data = $this->getAppInfoByPath($file, $lang);
+
+               if ($lang === null) {
+                       $this->appInfos[$appId] = $data;
+               }
+
+               return $data;
+       }
+
+       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;
-               }
-
                return $data;
        }
 

diff --git a/lib/private/Installer.php b/lib/private/Installer.php
index d5500c07a3cf9b7ef09e13517a23a04193e2306c..00fdd84c1bc833f12679f988c92589e20ec6d583 100644 (file)
--- a/lib/private/Installer.php
+++ b/lib/private/Installer.php
@@ -65,7 +65,7 @@ class Installer {
                }
 
                $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 a9f8b24d8317f35609abdb3b0871dfa1f80d2554..6afd4086cb3eb5503b306d2daffc9baf1d28fad4 100644 (file)
--- a/lib/private/legacy/OC_App.php
+++ b/lib/private/legacy/OC_App.php
@@ -313,7 +313,8 @@ class OC_App {
         * @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 @@ class OC_App {
         */
        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 1182f611b299fc472f80276fa2c0839ede19a32c..0af7cdfc495307308ac08fe0f6aaf47c4d9baec8 100644 (file)
--- 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".
         *