From 5118acce1dfcdb09cbc0f73927bf51c46feeaf38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Thu, 13 Jan 2022 10:34:47 -0300 Subject: [PATCH 1/3] Escape config-form's action attribute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- templates/config/form_display/form_top.twig | 2 +- test/classes/Config/FormDisplayTemplateTest.php | 6 +++--- test/classes/Config/PageSettingsTest.php | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/templates/config/form_display/form_top.twig b/templates/config/form_display/form_top.twig index f18a319857..9afc5e4928 100644 --- a/templates/config/form_display/form_top.twig +++ b/templates/config/form_display/form_top.twig @@ -1,4 +1,4 @@ -
+ {% if has_check_page_refresh %} diff --git a/test/classes/Config/FormDisplayTemplateTest.php b/test/classes/Config/FormDisplayTemplateTest.php index b5fca909f8..aadd6b42c7 100644 --- a/test/classes/Config/FormDisplayTemplateTest.php +++ b/test/classes/Config/FormDisplayTemplateTest.php @@ -38,13 +38,13 @@ class FormDisplayTemplateTest extends AbstractTestCase */ public function testDisplayFormTop(): void { - $_SERVER['REQUEST_URI'] = 'https://www.phpmyadmin.net'; + $_SERVER['REQUEST_URI'] = 'https://www.phpmyadmin.net/index.php?key=value&key2=">value2'; $GLOBALS['cfg']['ServerDefault'] = ''; $result = $this->formDisplayTemplate->displayFormTop(null, 'posted', [1]); $this->assertStringContainsString( - '', + '', $result ); diff --git a/test/classes/Config/PageSettingsTest.php b/test/classes/Config/PageSettingsTest.php index 8282e44721..15b5bbae84 100644 --- a/test/classes/Config/PageSettingsTest.php +++ b/test/classes/Config/PageSettingsTest.php @@ -54,7 +54,7 @@ class PageSettingsTest extends AbstractTestCase '
' . '
' . '', $html ); From 44eb12f15a562718bbe54c9a16af91ceea335d59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Thu, 13 Jan 2022 14:33:29 -0300 Subject: [PATCH 2/3] Improve query params filtering in Setup pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note: 'action_done' was not used since c962ee4287b50af754b8fe72c0c4ef6cebf3c489. Signed-off-by: Maurício Meneghini Fauth --- .../Controllers/Setup/ConfigController.php | 9 ++-- .../Controllers/Setup/FormController.php | 5 +- .../Controllers/Setup/HomeController.php | 49 ++----------------- .../Controllers/Setup/ServersController.php | 24 ++++++--- libraries/classes/Setup/FormProcessing.php | 16 ++++-- phpstan-baseline.neon | 5 -- psalm-baseline.xml | 8 --- setup/index.php | 8 ++- templates/setup/servers/index.twig | 6 ++- 9 files changed, 48 insertions(+), 82 deletions(-) diff --git a/libraries/classes/Controllers/Setup/ConfigController.php b/libraries/classes/Controllers/Setup/ConfigController.php index 5faa3bc66d..12e845fbee 100644 --- a/libraries/classes/Controllers/Setup/ConfigController.php +++ b/libraries/classes/Controllers/Setup/ConfigController.php @@ -5,8 +5,8 @@ declare(strict_types=1); namespace PhpMyAdmin\Controllers\Setup; use PhpMyAdmin\Config\FormDisplayTemplate; -use PhpMyAdmin\Core; use PhpMyAdmin\Setup\ConfigGenerator; +use function is_string; class ConfigController extends AbstractController { @@ -17,6 +17,9 @@ class ConfigController extends AbstractController */ public function index(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(); $formDisplayTemplate = new FormDisplayTemplate($GLOBALS['PMA_Config']); @@ -34,13 +37,13 @@ class ConfigController extends AbstractController $config = ConfigGenerator::getConfigFile($this->config); return $this->template->render('setup/config/index', [ - 'formset' => $params['formset'] ?? '', + 'formset' => $formset, 'pages' => $pages, 'form_top_html' => $formTop, 'fieldset_top_html' => $fieldsetTop, 'form_bottom_html' => $formBottom, 'fieldset_bottom_html' => $fieldsetBottom, - 'eol' => Core::ifSetOr($params['eol'], 'unix'), + 'eol' => $eol, 'config' => $config, ]); } diff --git a/libraries/classes/Controllers/Setup/FormController.php b/libraries/classes/Controllers/Setup/FormController.php index 271fce65e9..bd020c0ffa 100644 --- a/libraries/classes/Controllers/Setup/FormController.php +++ b/libraries/classes/Controllers/Setup/FormController.php @@ -10,6 +10,7 @@ use PhpMyAdmin\Core; use PhpMyAdmin\Setup\FormProcessing; use function ob_get_clean; use function ob_start; +use function is_string; class FormController extends AbstractController { @@ -22,7 +23,7 @@ class FormController extends AbstractController { $pages = $this->getPages(); - $formset = Core::isValid($params['formset'], 'scalar') ? $params['formset'] : null; + $formset = isset($params['formset']) && is_string($params['formset']) ? $params['formset'] : ''; $formClass = SetupFormList::get($formset); if ($formClass === null) { @@ -36,7 +37,7 @@ class FormController extends AbstractController $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 index 8fa13a0968..4c4c3ed005 100644 --- a/libraries/classes/Controllers/Setup/HomeController.php +++ b/libraries/classes/Controllers/Setup/HomeController.php @@ -9,10 +9,8 @@ use PhpMyAdmin\Config\FormDisplayTemplate; use PhpMyAdmin\Config\ServerConfigChecks; use PhpMyAdmin\Core; use PhpMyAdmin\LanguageManager; -use PhpMyAdmin\Sanitize; use PhpMyAdmin\Setup\Index; -use function preg_replace; -use function uniqid; +use function is_string; class HomeController extends AbstractController { @@ -23,11 +21,9 @@ class HomeController extends AbstractController */ public function index(array $params): string { - $pages = $this->getPages(); + $formset = isset($params['formset']) && is_string($params['formset']) ? $params['formset'] : ''; - // Handle done action info - $actionDone = Core::isValid($params['action_done'], 'scalar') ? $params['action_done'] : ''; - $actionDone = preg_replace('/[^a-z_]/', '', $actionDone); + $pages = $this->getPages(); // message handling Index::messagesBegin(); @@ -53,43 +49,6 @@ class HomeController extends AbstractController $text .= ''; 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(); @@ -205,7 +164,7 @@ class HomeController extends AbstractController ); return $this->template->render('setup/home/index', [ - 'formset' => $params['formset'] ?? '', + 'formset' => $formset, 'languages' => $languages, 'messages' => $messages, 'servers_form_top_html' => $serversFormTopHtml, diff --git a/libraries/classes/Controllers/Setup/ServersController.php b/libraries/classes/Controllers/Setup/ServersController.php index ae1fb52b5e..f0b78bbf83 100644 --- a/libraries/classes/Controllers/Setup/ServersController.php +++ b/libraries/classes/Controllers/Setup/ServersController.php @@ -5,10 +5,12 @@ declare(strict_types=1); namespace PhpMyAdmin\Controllers\Setup; use PhpMyAdmin\Config\Forms\Setup\ServersForm; -use PhpMyAdmin\Core; use PhpMyAdmin\Setup\FormProcessing; use function ob_get_clean; use function ob_start; +use function is_string; +use function is_numeric; +use function in_array; class ServersController extends AbstractController { @@ -19,12 +21,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 = Core::isValid($params['id'], 'numeric') ? (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; } @@ -33,10 +41,10 @@ class ServersController extends AbstractController $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, @@ -48,9 +56,9 @@ class ServersController extends AbstractController */ public function destroy(array $params): void { - $id = Core::isValid($params['id'], 'numeric') ? (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 index 5de581a888..5c06623af0 100644 --- a/libraries/classes/Setup/FormProcessing.php +++ b/libraries/classes/Setup/FormProcessing.php @@ -8,10 +8,12 @@ declare(strict_types=1); namespace PhpMyAdmin\Setup; use PhpMyAdmin\Config\FormDisplay; -use PhpMyAdmin\Core; use PhpMyAdmin\Response; use PhpMyAdmin\Template; use PhpMyAdmin\Url; +use function in_array; +use function is_string; +use function is_numeric; /** * PhpMyAdmin\Setup\FormProcessing class @@ -52,10 +54,14 @@ class FormProcessing } // form has errors, show warning - $page = $_GET['page'] ?? ''; - $formset = $_GET['formset'] ?? ''; - $formId = Core::isValid($_GET['id'], 'numeric') ? $_GET['id'] : ''; - 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 index 4f47d81dfc..dfdf327e12 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -270,11 +270,6 @@ parameters: count: 1 path: libraries/classes/Controllers/Setup/HomeController.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 index 3dba41f892..1589f01c73 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -446,9 +446,6 @@ - - $formset - new $formClass($this->config) @@ -461,11 +458,6 @@ PMA_IS_WINDOWS - - - $id - - $where_clause ?? null diff --git a/setup/index.php b/setup/index.php index 8439b1159c..2d069e3165 100644 --- a/setup/index.php +++ b/setup/index.php @@ -27,10 +27,9 @@ if (@file_exists(CONFIG_FILE) && ! $cfg['DBG']['demo']) { Core::fatalError(__('Configuration already exists, setup is disabled!')); } -$page = Core::isValid($_GET['page'], 'scalar') ? (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(); @@ -77,6 +76,5 @@ if ($page === 'servers') { $controller = new HomeController($GLOBALS['ConfigFile'], new Template()); echo $controller->index([ 'formset' => $_GET['formset'] ?? null, - 'action_done' => $_GET['action_done'] ?? null, 'version_check' => $_GET['version_check'] ?? null, ]); diff --git a/templates/setup/servers/index.twig b/templates/setup/servers/index.twig index 42b0745bfd..e6cb79a0ba 100644 --- a/templates/setup/servers/index.twig +++ b/templates/setup/servers/index.twig @@ -11,6 +11,10 @@

{% trans 'Add a new server' %}

{% endif %} -{{ page|raw }} +{% if mode == 'add' or mode == 'edit' or mode == 'revert' %} + {{ page|raw }} +{% else %} +

{% trans 'Something went wrong.' %}

+{% endif %} {% endblock %} From ca54f1db050859eb8555875c6aa5d7796fdf4b32 Mon Sep 17 00:00:00 2001 From: William Desportes Date: Thu, 13 Jan 2022 02:59:41 +0100 Subject: [PATCH 3/3] security - Fix - 2FA/U2F can be disabled without any code change Signed-off-by: William Desportes --- libraries/classes/DatabaseInterface.php | 27 +++++++++++++++---------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/libraries/classes/DatabaseInterface.php b/libraries/classes/DatabaseInterface.php index 47e1946c89..7ab5d754fb 100644 --- a/libraries/classes/DatabaseInterface.php +++ b/libraries/classes/DatabaseInterface.php @@ -1563,17 +1563,22 @@ class DatabaseInterface */ public function initRelationParamsCache() { - if (strlen($GLOBALS['db'])) { - $cfgRelation = $this->relation->getRelationsParam(); - if (empty($cfgRelation['db'])) { - $this->relation->fixPmaTables($GLOBALS['db'], false); - } - } - $cfgRelation = $this->relation->getRelationsParam(); - if (empty($cfgRelation['db']) && isset($GLOBALS['dblist'])) { - if ($GLOBALS['dblist']->databases->exists('phpmyadmin')) { - $this->relation->fixPmaTables('phpmyadmin', false); - } + $storageDbName = $GLOBALS['cfg']['Server']['pmadb'] ?? ''; + // Use "phpmyadmin" as a default database name to check to keep the behavior consistent + $storageDbName = $storageDbName !== null + && 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->relation->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)) { + $this->relation->fixPmaTables($GLOBALS['db'], false); } }