Improve security of UpgradeController and related functionality. (#3387)

vufind-org · Feb 7, 2024 · a19577d · a19577d
1 parent 51f2dda
commit a19577d
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 11 deletions.
diff --git a/module/VuFind/src/VuFind/Config/Upgrade.php b/module/VuFind/src/VuFind/Config/Upgrade.php
@@ -1358,6 +1358,8 @@ protected function upgradeILS()
             $this->addWarning('WARNING: Could not find ILS driver setting.');
         } elseif ('Sample' == $driver) {
             // No configuration file for Sample driver
+        } elseif ('AdminScripts' == $driver) {
+            // Prevent abuse if upgrade process is hijacked
         } elseif (!file_exists($this->oldDir . '/' . $driver . '.ini')) {
             $this->addWarning(
                 "WARNING: Could not find {$driver}.ini file; "

diff --git a/module/VuFind/src/VuFind/Controller/UpgradeController.php b/module/VuFind/src/VuFind/Controller/UpgradeController.php
@@ -197,9 +197,7 @@ public function errorAction()
     public function establishversionsAction()
     {
         $this->cookie->newVersion = Version::getBuildVersion();
-        $this->cookie->oldVersion = Version::getBuildVersion(
-            $this->cookie->sourceDir
-        );
+        $this->cookie->oldVersion = Version::getBuildVersion($this->getSourceDir());
 
         // Block upgrade when encountering common errors:
         if (empty($this->cookie->oldVersion)) {
@@ -234,7 +232,7 @@ public function fixconfigAction()
     {
         $localConfig = dirname($this->getForcedLocalConfigPath('config.ini'));
         $confDir = $this->cookie->oldVersion < 2
-            ? $this->cookie->sourceDir . '/web/conf'
+            ? $this->getSourceDir() . '/web/conf'
             : $localConfig;
         $upgrader = new Upgrade(
             $this->cookie->oldVersion,
@@ -809,7 +807,7 @@ public function getsourcedirAction()
         // Process form submission:
         $dir = $this->params()->fromPost('sourcedir');
         if (!empty($dir)) {
-            if (!is_dir($dir)) {
+            if (!$this->isSourceDirValid($dir)) {
                 $this->flashMessenger()
                     ->addMessage($dir . ' does not exist.', 'error');
             } elseif (!file_exists($dir . '/build.xml')) {
@@ -819,7 +817,7 @@ public function getsourcedirAction()
                     'error'
                 );
             } else {
-                $this->cookie->sourceDir = rtrim($dir, '\/');
+                $this->setSourceDir(rtrim($dir, '\/'));
                 // Clear out request to avoid infinite loop:
                 $this->getRequest()->getPost()->set('sourcedir', '');
                 return $this->forwardTo('Upgrade', 'Home');
@@ -864,7 +862,7 @@ public function getsourceversionAction()
                 );
             } else {
                 $this->cookie->oldVersion = $version;
-                $this->cookie->sourceDir = realpath(APPLICATION_PATH);
+                $this->setSourceDir(realpath(APPLICATION_PATH));
                 // Clear out request to avoid infinite loop:
                 $this->getRequest()->getPost()->set('sourceversion', '');
                 $this->processSkipParam();
@@ -889,6 +887,50 @@ protected function performCriticalChecks()
             ?? null;
     }
 
+    /**
+     * Validate a source directory string.
+     *
+     * @param string $dir Directory string to check
+     *
+     * @return bool
+     */
+    protected function isSourceDirValid(string $dir): bool
+    {
+        // Prevent abuse of stream wrappers:
+        if (empty($dir) || str_contains($dir, '://')) {
+            return false;
+        }
+        return is_dir($dir);
+    }
+
+    /**
+     * Set the source directory for the upgrade
+     *
+     * @param string $dir Directory to set
+     *
+     * @return void
+     */
+    protected function setSourceDir(string $dir): void
+    {
+        $this->cookie->sourceDir = $dir;
+    }
+
+    /**
+     * Get the source directory for the upgrade
+     *
+     * @param bool $validate Should we validate the directory?
+     *
+     * @return string
+     */
+    protected function getSourceDir($validate = true): string
+    {
+        $sourceDir = $this->cookie->sourceDir ?? '';
+        if ($validate && !$this->isSourceDirValid($sourceDir)) {
+            throw new \Exception('Unexpected source directory value!');
+        }
+        return $sourceDir;
+    }
+
     /**
      * Display summary of installation status
      *
@@ -904,10 +946,7 @@ public function homeAction()
         }
 
         // First find out which version we are upgrading:
-        if (
-            !isset($this->cookie->sourceDir)
-            || !is_dir($this->cookie->sourceDir)
-        ) {
+        if (!$this->isSourceDirValid($this->getSourceDir(false))) {
             return $this->forwardTo('Upgrade', 'GetSourceDir');
         }