Merge branch 'QA_5_1-security' into master-security

Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>
phpmyadmin · Jan 13, 2022 · 9b3a1f7 · 9b3a1f7
2 parents 0bee94b + 813ffbf
commit 9b3a1f7
Show file tree

Hide file tree

Showing 12 changed files with 63 additions and 96 deletions.
diff --git a/libraries/classes/ConfigStorage/Relation.php b/libraries/classes/ConfigStorage/Relation.php
@@ -1784,15 +1784,22 @@ public function getConfigurationStorageDbName(): string
      */
     public function initRelationParamsCache(): void
     {
-        if (strlen($GLOBALS['db'])) {
-            $relationParameters = $this->getRelationParameters();
-            if ($relationParameters->db === null) {
-                $this->fixPmaTables($GLOBALS['db'], false);
-            }
-        }
-
         $storageDbName = $GLOBALS['cfg']['Server']['pmadb'] ?? '';
         // Use "phpmyadmin" as a default database name to check to keep the behavior consistent
-        $this->fixPmaTables($storageDbName ?: 'phpmyadmin', false);
+        $storageDbName = is_string($storageDbName) && $storageDbName !== '' ? $storageDbName : 'phpmyadmin';
+
+        // This will make users not having explicitly listed databases
+        // have config values filled by the default phpMyAdmin storage table name values
+        $this->fixPmaTables($storageDbName, false);
+
+        // This global will be changed if fixPmaTables did find one valid table
+        $storageDbName = $GLOBALS['cfg']['Server']['pmadb'] ?? '';
+
+        // Empty means that until now no pmadb was found eligible
+        if (! empty($storageDbName)) {
+            return;
+        }
+
+        $this->fixPmaTables($GLOBALS['db'], false);
     }
 }
diff --git a/libraries/classes/Controllers/Setup/ConfigController.php b/libraries/classes/Controllers/Setup/ConfigController.php
@@ -6,7 +6,7 @@
 
 use PhpMyAdmin\Setup\ConfigGenerator;
 
-use function is_scalar;
+use function is_string;
 
 class ConfigController extends AbstractController
 {
@@ -17,6 +17,9 @@ class ConfigController extends AbstractController
      */
     public function __invoke(array $params): string
     {
+        $formset = isset($params['formset']) && is_string($params['formset']) ? $params['formset'] : '';
+        $eol = isset($params['eol']) && $params['eol'] === 'win' ? 'win' : 'unix';
+
         $pages = $this->getPages();
 
         static $hasCheckPageRefresh = false;
@@ -27,9 +30,9 @@ public function __invoke(array $params): string
         $config = ConfigGenerator::getConfigFile($this->config);
 
         return $this->template->render('setup/config/index', [
-            'formset' => $params['formset'] ?? '',
+            'formset' => $formset,
             'pages' => $pages,
-            'eol' => isset($params['eol']) && is_scalar($params['eol']) ? $params['eol'] : 'unix',
+            'eol' => $eol,
             'config' => $config,
             'has_check_page_refresh' => $hasCheckPageRefresh,
         ]);

diff --git a/libraries/classes/Controllers/Setup/FormController.php b/libraries/classes/Controllers/Setup/FormController.php
@@ -10,7 +10,7 @@
 use PhpMyAdmin\Setup\FormProcessing;
 
 use function __;
-use function is_scalar;
+use function is_string;
 use function ob_get_clean;
 use function ob_start;
 
@@ -25,7 +25,7 @@ public function __invoke(array $params): string
     {
         $pages = $this->getPages();
 
-        $formset = isset($params['formset']) && is_scalar($params['formset']) ? (string) $params['formset'] : '';
+        $formset = isset($params['formset']) && is_string($params['formset']) ? $params['formset'] : '';
 
         $formClass = SetupFormList::get($formset);
         if ($formClass === null) {
@@ -39,7 +39,7 @@ public function __invoke(array $params): string
         $page = ob_get_clean();
 
         return $this->template->render('setup/form/index', [
-            'formset' => $params['formset'] ?? '',
+            'formset' => $formset,
             'pages' => $pages,
             'name' => $form::getName(),
             'page' => $page,

diff --git a/libraries/classes/Controllers/Setup/HomeController.php b/libraries/classes/Controllers/Setup/HomeController.php
@@ -6,14 +6,12 @@
 
 use PhpMyAdmin\Config\ServerConfigChecks;
 use PhpMyAdmin\LanguageManager;
-use PhpMyAdmin\Sanitize;
 use PhpMyAdmin\Setup\Index;
 
 use function __;
 use function array_keys;
 use function is_scalar;
-use function preg_replace;
-use function uniqid;
+use function is_string;
 
 class HomeController extends AbstractController
 {
@@ -24,13 +22,9 @@ class HomeController extends AbstractController
      */
     public function __invoke(array $params): string
     {
-        $pages = $this->getPages();
+        $formset = isset($params['formset']) && is_string($params['formset']) ? $params['formset'] : '';
 
-        // Handle done action info
-        $actionDone = isset($params['action_done']) && is_scalar($params['action_done'])
-            ? (string) $params['action_done']
-            : '';
-        $actionDone = preg_replace('/[^a-z_]/', '', $actionDone);
+        $pages = $this->getPages();
 
         // message handling
         Index::messagesBegin();
@@ -56,43 +50,6 @@ public function __invoke(array $params): string
         $text .= '</a>';
         Index::messagesSet('notice', 'no_https', __('Insecure connection'), $text);
 
-        // Check for done action info and set notice message if present
-        switch ($actionDone) {
-            case 'config_saved':
-                /* Use uniqid to display this message every time configuration is saved */
-                Index::messagesSet(
-                    'notice',
-                    uniqid('config_saved'),
-                    __('Configuration saved.'),
-                    Sanitize::sanitizeMessage(
-                        __(
-                            'Configuration saved to file config/config.inc.php in phpMyAdmin '
-                            . 'top level directory, copy it to top level one and delete '
-                            . 'directory config to use it.'
-                        )
-                    )
-                );
-                break;
-            case 'config_not_saved':
-                /* Use uniqid to display this message every time configuration is saved */
-                Index::messagesSet(
-                    'notice',
-                    uniqid('config_not_saved'),
-                    __('Configuration not saved!'),
-                    Sanitize::sanitizeMessage(
-                        __(
-                            'Please create web server writable folder [em]config[/em] in '
-                            . 'phpMyAdmin top level directory as described in '
-                            . '[doc@setup_script]documentation[/doc]. Otherwise you will be '
-                            . 'only able to download or display it.'
-                        )
-                    )
-                );
-                break;
-            default:
-                break;
-        }
-
         Index::messagesEnd();
         $messages = Index::messagesShowHtml();
 
@@ -136,7 +93,7 @@ public function __invoke(array $params): string
         }
 
         return $this->template->render('setup/home/index', [
-            'formset' => $params['formset'] ?? '',
+            'formset' => $formset,
             'languages' => $languages,
             'messages' => $messages,
             'server_count' => $this->config->getServerCount(),

diff --git a/libraries/classes/Controllers/Setup/ServersController.php b/libraries/classes/Controllers/Setup/ServersController.php
@@ -7,7 +7,9 @@
 use PhpMyAdmin\Config\Forms\Setup\ServersForm;
 use PhpMyAdmin\Setup\FormProcessing;
 
+use function in_array;
 use function is_numeric;
+use function is_string;
 use function ob_get_clean;
 use function ob_start;
 
@@ -20,12 +22,18 @@ class ServersController extends AbstractController
      */
     public function index(array $params): string
     {
+        $formset = isset($params['formset']) && is_string($params['formset']) ? $params['formset'] : '';
+        $id = isset($params['id']) && is_numeric($params['id']) && (int) $params['id'] >= 1 ? (int) $params['id'] : 0;
+        $mode = '';
+        if (isset($params['mode']) && in_array($params['mode'], ['add', 'edit', 'revert'], true)) {
+            $mode = $params['mode'];
+        }
+
         $pages = $this->getPages();
 
-        $id = isset($params['id']) && is_numeric($params['id']) ? (int) $params['id'] : null;
-        $hasServer = ! empty($id) && $this->config->get('Servers/' . $id) !== null;
+        $hasServer = $id >= 1 && $this->config->get('Servers/' . $id) !== null;
 
-        if (! $hasServer && ($params['mode'] !== 'revert' && $params['mode'] !== 'edit')) {
+        if (! $hasServer && $mode !== 'revert' && $mode !== 'edit') {
             $id = 0;
         }
 
@@ -34,10 +42,10 @@ public function index(array $params): string
         $page = ob_get_clean();
 
         return $this->template->render('setup/servers/index', [
-            'formset' => $params['formset'] ?? '',
+            'formset' => $formset,
             'pages' => $pages,
             'has_server' => $hasServer,
-            'mode' => $params['mode'],
+            'mode' => $mode,
             'server_id' => $id,
             'server_dsn' => $this->config->getServerDSN($id),
             'page' => $page,
@@ -49,9 +57,9 @@ public function index(array $params): string
      */
     public function destroy(array $params): void
     {
-        $id = isset($params['id']) && is_numeric($params['id']) ? (int) $params['id'] : null;
+        $id = isset($params['id']) && is_numeric($params['id']) && (int) $params['id'] >= 1 ? (int) $params['id'] : 0;
 
-        $hasServer = ! empty($id) && $this->config->get('Servers/' . $id) !== null;
+        $hasServer = $id >= 1 && $this->config->get('Servers/' . $id) !== null;
 
         if (! $hasServer) {
             return;

diff --git a/libraries/classes/Setup/FormProcessing.php b/libraries/classes/Setup/FormProcessing.php
@@ -12,7 +12,9 @@
 use PhpMyAdmin\Template;
 use PhpMyAdmin\Url;
 
+use function in_array;
 use function is_numeric;
+use function is_string;
 
 /**
  * PhpMyAdmin\Setup\FormProcessing class
@@ -51,10 +53,14 @@ public static function process(FormDisplay $form_display): void
         }
 
         // form has errors, show warning
-        $page = $_GET['page'] ?? '';
-        $formset = $_GET['formset'] ?? '';
-        $formId = isset($_GET['id']) && is_numeric($_GET['id']) ? (int) $_GET['id'] : null;
-        if ($formId === null && $page === 'servers') {
+        $page = 'index';
+        if (isset($_GET['page']) && in_array($_GET['page'], ['form', 'config', 'servers'], true)) {
+            $page = $_GET['page'];
+        }
+
+        $formset = isset($_GET['formset']) && is_string($_GET['formset']) ? $_GET['formset'] : '';
+        $formId = isset($_GET['id']) && is_numeric($_GET['id']) && (int) $_GET['id'] >= 1 ? (int) $_GET['id'] : 0;
+        if ($formId === 0 && $page === 'servers') {
             // we've just added a new server, get its id
             $formId = $form_display->getConfigFile()->getServerCount();
         }

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -1455,11 +1455,6 @@ parameters:
 			count: 1
 			path: libraries/classes/Controllers/Setup/ServersController.php
 
-		-
-			message: "#^Parameter \\#1 \\$server of method PhpMyAdmin\\\\Config\\\\ConfigFile\\:\\:getServerDSN\\(\\) expects int, int\\|null given\\.$#"
-			count: 1
-			path: libraries/classes/Controllers/Setup/ServersController.php
-
 		-
 			message: "#^Comparison operation \"\\>\" between 0 and 0 is always false\\.$#"
 			count: 1

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -2716,11 +2716,6 @@
       <code>$id</code>
     </MixedArgumentTypeCoercion>
   </file>
-  <file src="libraries/classes/Controllers/Setup/ServersController.php">
-    <PossiblyNullArgument occurrences="1">
-      <code>$id</code>
-    </PossiblyNullArgument>
-  </file>
   <file src="libraries/classes/Controllers/Sql/ColumnPreferencesController.php">
     <MixedArgument occurrences="2">
       <code>$db</code>
@@ -12681,12 +12676,6 @@
       <code>self::getServerPart($cf, $crlf, $conf['Servers'])</code>
     </PossiblyNullOperand>
   </file>
-  <file src="libraries/classes/Setup/FormProcessing.php">
-    <MixedAssignment occurrences="2">
-      <code>$formset</code>
-      <code>$page</code>
-    </MixedAssignment>
-  </file>
   <file src="libraries/classes/Setup/Index.php">
     <MixedArgument occurrences="6">
       <code>$date</code>

diff --git a/setup/index.php b/setup/index.php
@@ -32,10 +32,9 @@
     Core::fatalError(__('Configuration already exists, setup is disabled!'));
 }
 
-$page = isset($_GET['page']) && is_scalar($_GET['page']) ? (string) $_GET['page'] : '';
-$page = preg_replace('/[^a-z]/', '', $page);
-if ($page === '') {
-    $page = 'index';
+$page = 'index';
+if (isset($_GET['page']) && in_array($_GET['page'], ['form', 'config', 'servers'], true)) {
+    $page = $_GET['page'];
 }
 
 Core::noCacheHeader();
@@ -79,6 +78,5 @@
 
 echo (new HomeController($GLOBALS['ConfigFile'], new Template()))([
     'formset' => $_GET['formset'] ?? null,
-    'action_done' => $_GET['action_done'] ?? null,
     'version_check' => $_GET['version_check'] ?? null,
 ]);
diff --git a/templates/config/form_display/display.twig b/templates/config/form_display/display.twig
@@ -1,4 +1,4 @@
-<form method="post" action="{{ action|raw }}" class="config-form disableAjax">
+<form method="post" action="{{ action|e('html_attr') }}" class="config-form disableAjax">
   <input type="hidden" name="tab_hash" value="">
   {% if has_check_page_refresh %}
     <input type="hidden" name="check_page_refresh" id="check_page_refresh" value="">

diff --git a/templates/setup/servers/index.twig b/templates/setup/servers/index.twig
@@ -11,6 +11,10 @@
   <h2>{% trans 'Add a new server' %}</h2>
 {% endif %}
 
-{{ page|raw }}
+{% if mode == 'add' or mode == 'edit' or mode == 'revert' %}
+  {{ page|raw }}
+{% else %}
+  <p>{% trans 'Something went wrong.' %}</p>
+{% endif %}
 
 {% endblock %}
diff --git a/test/classes/Config/PageSettingsTest.php b/test/classes/Config/PageSettingsTest.php
@@ -53,7 +53,7 @@ public function testShowGroupBrowse(): void
             '<div id="page_settings_modal">'
             . '<div class="page_settings">'
             . '<form method="post" '
-            . 'action="index.php?db=db&server=1&lang=en" '
+            . 'action="index.php&#x3F;db&#x3D;db&amp;server&#x3D;1&amp;lang&#x3D;en" '
             . 'class="config-form disableAjax">',
             $html
         );