From f940693c75200ee2ae08de39a046af9a182280a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Mon, 7 Mar 2022 22:59:33 -0300 Subject: [PATCH] Move `Util::checkParameters` method to the `AbstractController` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- .../Controllers/AbstractController.php | 42 ++++++++++ .../Database/DataDictionaryController.php | 2 +- .../Database/DesignerController.php | 2 +- .../Controllers/Database/EventsController.php | 2 +- .../Controllers/Database/ExportController.php | 2 +- .../Controllers/Database/ImportController.php | 2 +- .../Operations/CollationController.php | 2 +- .../Database/OperationsController.php | 2 +- .../Database/QueryByExampleController.php | 2 +- .../Database/RoutinesController.php | 4 +- .../Controllers/Database/SearchController.php | 2 +- .../Controllers/Database/SqlController.php | 2 +- .../Structure/FavoriteTableController.php | 2 +- .../Structure/RealRowCountController.php | 2 +- .../Database/StructureController.php | 2 +- .../Database/TrackingController.php | 2 +- .../Database/TriggersController.php | 4 +- .../Controllers/Export/ExportController.php | 2 +- .../Controllers/Import/ImportController.php | 3 +- .../Controllers/SchemaExportController.php | 12 ++- .../classes/Controllers/Sql/SqlController.php | 3 +- .../Controllers/Table/AddFieldController.php | 5 +- .../Controllers/Table/ChartController.php | 6 +- .../Controllers/Table/CreateController.php | 5 +- .../Table/DeleteConfirmController.php | 2 +- .../DropColumnConfirmationController.php | 2 +- .../Controllers/Table/ExportController.php | 2 +- .../Table/FindReplaceController.php | 2 +- .../Controllers/Table/GetFieldController.php | 6 +- .../Table/GisVisualizationController.php | 2 +- .../Controllers/Table/ImportController.php | 2 +- .../Table/IndexRenameController.php | 2 +- .../Controllers/Table/IndexesController.php | 2 +- .../Table/OperationsController.php | 2 +- .../Controllers/Table/ReplaceController.php | 2 +- .../Controllers/Table/SearchController.php | 2 +- .../Controllers/Table/SqlController.php | 2 +- .../Table/Structure/ChangeController.php | 2 + .../Table/Structure/PrimaryController.php | 2 +- .../Controllers/Table/StructureController.php | 2 +- .../Controllers/Table/TrackingController.php | 2 +- .../Controllers/Table/TriggersController.php | 4 +- .../Table/ZoomSearchController.php | 2 +- .../Controllers/View/CreateController.php | 2 +- .../Controllers/View/OperationsController.php | 2 +- libraries/classes/Table/ColumnsDefinition.php | 7 -- libraries/classes/Util.php | 42 ---------- .../Controllers/AbstractControllerTest.php | 83 +++++++++++++++++++ test/classes/UtilTest.php | 48 ----------- 49 files changed, 188 insertions(+), 154 deletions(-) create mode 100644 test/classes/Controllers/AbstractControllerTest.php diff --git a/libraries/classes/Controllers/AbstractController.php b/libraries/classes/Controllers/AbstractController.php index 8b3cf58189..85ee1c0140 100644 --- a/libraries/classes/Controllers/AbstractController.php +++ b/libraries/classes/Controllers/AbstractController.php @@ -5,12 +5,14 @@ declare(strict_types=1); namespace PhpMyAdmin\Controllers; use PhpMyAdmin\Core; +use PhpMyAdmin\Html\MySQLDocumentation; use PhpMyAdmin\Message; use PhpMyAdmin\ResponseRenderer; use PhpMyAdmin\Template; use PhpMyAdmin\Url; use function __; +use function basename; use function defined; use function strlen; @@ -101,4 +103,44 @@ abstract class AbstractController $uri = './index.php?route=' . $route . Url::getCommonRaw($params, '&'); Core::sendHeaderLocation($uri); } + + /** + * Function added to avoid path disclosures. + * Called by each script that needs parameters, it displays + * an error message and, by default, stops the execution. + * + * @param bool $request Check parameters in request + * @psalm-param non-empty-list $params The names of the parameters needed by the calling script + */ + protected function checkParameters(array $params, bool $request = false): void + { + $reportedScriptName = basename($GLOBALS['PMA_PHP_SELF']); + $foundError = false; + $errorMessage = ''; + if ($request) { + $array = $_REQUEST; + } else { + $array = $GLOBALS; + } + + foreach ($params as $param) { + if (isset($array[$param]) && $array[$param] !== '') { + continue; + } + + $errorMessage .= $reportedScriptName + . ': ' . __('Missing parameter:') . ' ' + . $param + . MySQLDocumentation::showDocumentation('faq', 'faqmissingparameters', true) + . '[br]'; + $foundError = true; + } + + if (! $foundError) { + return; + } + + $this->response->setHttpResponseCode(400); + Core::fatalError($errorMessage); + } } diff --git a/libraries/classes/Controllers/Database/DataDictionaryController.php b/libraries/classes/Controllers/Database/DataDictionaryController.php index 8eb1aa0f85..aaf7cd4cde 100644 --- a/libraries/classes/Controllers/Database/DataDictionaryController.php +++ b/libraries/classes/Controllers/Database/DataDictionaryController.php @@ -42,7 +42,7 @@ class DataDictionaryController extends AbstractController public function __invoke(): void { - Util::checkParameters(['db'], true); + $this->checkParameters(['db'], true); $relationParameters = $this->relation->getRelationParameters(); diff --git a/libraries/classes/Controllers/Database/DesignerController.php b/libraries/classes/Controllers/Database/DesignerController.php index 7d7688d9f7..36184123a8 100644 --- a/libraries/classes/Controllers/Database/DesignerController.php +++ b/libraries/classes/Controllers/Database/DesignerController.php @@ -140,7 +140,7 @@ class DesignerController extends AbstractController return; } - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/EventsController.php b/libraries/classes/Controllers/Database/EventsController.php index 1ed8da239b..2af8ac4264 100644 --- a/libraries/classes/Controllers/Database/EventsController.php +++ b/libraries/classes/Controllers/Database/EventsController.php @@ -38,7 +38,7 @@ final class EventsController extends AbstractController $this->addScriptFiles(['database/events.js']); if (! $this->response->isAjax()) { - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/ExportController.php b/libraries/classes/Controllers/Database/ExportController.php index f9fbe56224..2d8ee819f6 100644 --- a/libraries/classes/Controllers/Database/ExportController.php +++ b/libraries/classes/Controllers/Database/ExportController.php @@ -50,7 +50,7 @@ final class ExportController extends AbstractController // /database/export, in which case we don't obey $cfg['MaxTableList'] $GLOBALS['sub_part'] = '_export'; - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/ImportController.php b/libraries/classes/Controllers/Database/ImportController.php index 5e6da2daa1..7eabeeb28b 100644 --- a/libraries/classes/Controllers/Database/ImportController.php +++ b/libraries/classes/Controllers/Database/ImportController.php @@ -42,7 +42,7 @@ final class ImportController extends AbstractController $this->addScriptFiles(['import.js']); - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/Operations/CollationController.php b/libraries/classes/Controllers/Database/Operations/CollationController.php index 07a8335546..7a033bd670 100644 --- a/libraries/classes/Controllers/Database/Operations/CollationController.php +++ b/libraries/classes/Controllers/Database/Operations/CollationController.php @@ -47,7 +47,7 @@ final class CollationController extends AbstractController return; } - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/OperationsController.php b/libraries/classes/Controllers/Database/OperationsController.php index acb123b0cf..ae73086c3b 100644 --- a/libraries/classes/Controllers/Database/OperationsController.php +++ b/libraries/classes/Controllers/Database/OperationsController.php @@ -237,7 +237,7 @@ class OperationsController extends AbstractController $this->relation->setDbComment($GLOBALS['db'], $_POST['comment']); } - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/QueryByExampleController.php b/libraries/classes/Controllers/Database/QueryByExampleController.php index 9757b490b0..75e5ae7e13 100644 --- a/libraries/classes/Controllers/Database/QueryByExampleController.php +++ b/libraries/classes/Controllers/Database/QueryByExampleController.php @@ -132,7 +132,7 @@ class QueryByExampleController extends AbstractController $GLOBALS['sub_part'] = '_qbe'; - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/RoutinesController.php b/libraries/classes/Controllers/Database/RoutinesController.php index 1f862fca14..7e5bb50cf0 100644 --- a/libraries/classes/Controllers/Database/RoutinesController.php +++ b/libraries/classes/Controllers/Database/RoutinesController.php @@ -52,7 +52,7 @@ class RoutinesController extends AbstractController * Displays the header and tabs */ if (! empty($GLOBALS['table']) && in_array($GLOBALS['table'], $this->dbi->getTables($GLOBALS['db']))) { - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); @@ -62,7 +62,7 @@ class RoutinesController extends AbstractController } else { $GLOBALS['table'] = ''; - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/SearchController.php b/libraries/classes/Controllers/Database/SearchController.php index 42b94e08e1..383f436629 100644 --- a/libraries/classes/Controllers/Database/SearchController.php +++ b/libraries/classes/Controllers/Database/SearchController.php @@ -35,7 +35,7 @@ class SearchController extends AbstractController 'makegrid.js', ]); - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/SqlController.php b/libraries/classes/Controllers/Database/SqlController.php index 15a47b805f..2d70d94673 100644 --- a/libraries/classes/Controllers/Database/SqlController.php +++ b/libraries/classes/Controllers/Database/SqlController.php @@ -41,7 +41,7 @@ class SqlController extends AbstractController $this->response->addHTML($pageSettings->getErrorHTML()); $this->response->addHTML($pageSettings->getHTML()); - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/Structure/FavoriteTableController.php b/libraries/classes/Controllers/Database/Structure/FavoriteTableController.php index 040d0eb005..95d4caeced 100644 --- a/libraries/classes/Controllers/Database/Structure/FavoriteTableController.php +++ b/libraries/classes/Controllers/Database/Structure/FavoriteTableController.php @@ -38,7 +38,7 @@ final class FavoriteTableController extends AbstractController 'sync_favorite_tables' => $_REQUEST['sync_favorite_tables'] ?? null, ]; - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/Structure/RealRowCountController.php b/libraries/classes/Controllers/Database/Structure/RealRowCountController.php index 40c0a876e4..19be7f2199 100644 --- a/libraries/classes/Controllers/Database/Structure/RealRowCountController.php +++ b/libraries/classes/Controllers/Database/Structure/RealRowCountController.php @@ -34,7 +34,7 @@ final class RealRowCountController extends AbstractController 'table' => $_REQUEST['table'] ?? null, ]; - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/StructureController.php b/libraries/classes/Controllers/Database/StructureController.php index 76188b64f1..f9742e3584 100644 --- a/libraries/classes/Controllers/Database/StructureController.php +++ b/libraries/classes/Controllers/Database/StructureController.php @@ -140,7 +140,7 @@ class StructureController extends AbstractController 'sort_order' => $_REQUEST['sort_order'] ?? null, ]; - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/TrackingController.php b/libraries/classes/Controllers/Database/TrackingController.php index e4e6e6fe8c..9e0dec958f 100644 --- a/libraries/classes/Controllers/Database/TrackingController.php +++ b/libraries/classes/Controllers/Database/TrackingController.php @@ -47,7 +47,7 @@ class TrackingController extends AbstractController { $this->addScriptFiles(['vendor/jquery/jquery.tablesorter.js', 'database/tracking.js']); - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Database/TriggersController.php b/libraries/classes/Controllers/Database/TriggersController.php index fce9c86333..434aa6f563 100644 --- a/libraries/classes/Controllers/Database/TriggersController.php +++ b/libraries/classes/Controllers/Database/TriggersController.php @@ -39,7 +39,7 @@ class TriggersController extends AbstractController * Displays the header and tabs */ if (! empty($GLOBALS['table']) && in_array($GLOBALS['table'], $this->dbi->getTables($GLOBALS['db']))) { - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); @@ -49,7 +49,7 @@ class TriggersController extends AbstractController } else { $GLOBALS['table'] = ''; - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Export/ExportController.php b/libraries/classes/Controllers/Export/ExportController.php index 8b99b1fa61..269102d27a 100644 --- a/libraries/classes/Controllers/Export/ExportController.php +++ b/libraries/classes/Controllers/Export/ExportController.php @@ -212,7 +212,7 @@ final class ExportController extends AbstractController $GLOBALS[$param] = $postParams[$param]; } - Util::checkParameters(['what', 'export_type']); + $this->checkParameters(['what', 'export_type']); // sanitize this parameter which will be used below in a file inclusion $GLOBALS['what'] = Core::securePath($whatParam); diff --git a/libraries/classes/Controllers/Import/ImportController.php b/libraries/classes/Controllers/Import/ImportController.php index 1e2fe4753e..e0cec17b5f 100644 --- a/libraries/classes/Controllers/Import/ImportController.php +++ b/libraries/classes/Controllers/Import/ImportController.php @@ -255,8 +255,7 @@ final class ImportController extends AbstractController Core::setPostAsGlobal($post_patterns); - // Check needed parameters - Util::checkParameters(['import_type', 'format']); + $this->checkParameters(['import_type', 'format']); // We don't want anything special in format $GLOBALS['format'] = Core::securePath($GLOBALS['format']); diff --git a/libraries/classes/Controllers/SchemaExportController.php b/libraries/classes/Controllers/SchemaExportController.php index f06306a950..52c181cc37 100644 --- a/libraries/classes/Controllers/SchemaExportController.php +++ b/libraries/classes/Controllers/SchemaExportController.php @@ -4,8 +4,11 @@ declare(strict_types=1); namespace PhpMyAdmin\Controllers; +use PhpMyAdmin\Core; use PhpMyAdmin\Export; -use PhpMyAdmin\Util; +use PhpMyAdmin\Html\MySQLDocumentation; + +use function __; /** * Schema export handler @@ -23,7 +26,12 @@ class SchemaExportController public function __invoke(): void { if (! isset($_POST['export_type'])) { - Util::checkParameters(['export_type']); + $errorMessage = __('Missing parameter:') . ' export_type' + . MySQLDocumentation::showDocumentation('faq', 'faqmissingparameters', true) + . '[br]'; + Core::fatalError($errorMessage); + + return; } /** diff --git a/libraries/classes/Controllers/Sql/SqlController.php b/libraries/classes/Controllers/Sql/SqlController.php index 9826d86fe7..233383afbc 100644 --- a/libraries/classes/Controllers/Sql/SqlController.php +++ b/libraries/classes/Controllers/Sql/SqlController.php @@ -124,8 +124,7 @@ class SqlController extends AbstractController // set $goto to what will be displayed if query returns 0 rows $GLOBALS['goto'] = ''; } else { - // Now we can check the parameters - Util::checkParameters(['sql_query']); + $this->checkParameters(['sql_query']); } /** diff --git a/libraries/classes/Controllers/Table/AddFieldController.php b/libraries/classes/Controllers/Table/AddFieldController.php index 1cd41d31ca..7007a800bd 100644 --- a/libraries/classes/Controllers/Table/AddFieldController.php +++ b/libraries/classes/Controllers/Table/AddFieldController.php @@ -61,8 +61,7 @@ class AddFieldController extends AbstractController { $this->addScriptFiles(['table/structure.js']); - // Check parameters - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $cfg = $this->config->settings; @@ -178,6 +177,8 @@ class AddFieldController extends AbstractController $this->addScriptFiles(['vendor/jquery/jquery.uitablefilter.js', 'indexes.js']); + $this->checkParameters(['server', 'db', 'table', 'num_fields']); + $templateData = $this->columnsDefinition->displayForm( '/table/add-field', $GLOBALS['num_fields'], diff --git a/libraries/classes/Controllers/Table/ChartController.php b/libraries/classes/Controllers/Table/ChartController.php index 90a73e447e..d5aa5bcb58 100644 --- a/libraries/classes/Controllers/Table/ChartController.php +++ b/libraries/classes/Controllers/Table/ChartController.php @@ -81,7 +81,7 @@ class ChartController extends AbstractController * Runs common work */ if (strlen($GLOBALS['table']) > 0) { - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $url_params = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); @@ -96,7 +96,7 @@ class ChartController extends AbstractController $url_params['goto'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $url_params['back'] = Url::getFromRoute('/sql'); - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); @@ -168,7 +168,7 @@ class ChartController extends AbstractController public function ajax(): void { if (strlen($GLOBALS['table']) > 0 && strlen($GLOBALS['db']) > 0) { - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Controllers/Table/CreateController.php b/libraries/classes/Controllers/Table/CreateController.php index 0ccf154b06..709874a854 100644 --- a/libraries/classes/Controllers/Table/CreateController.php +++ b/libraries/classes/Controllers/Table/CreateController.php @@ -15,7 +15,6 @@ use PhpMyAdmin\Table\ColumnsDefinition; use PhpMyAdmin\Template; use PhpMyAdmin\Transformations; use PhpMyAdmin\Url; -use PhpMyAdmin\Util; use function __; use function htmlspecialchars; @@ -58,7 +57,7 @@ class CreateController extends AbstractController public function __invoke(): void { - Util::checkParameters(['db']); + $this->checkParameters(['db']); $cfg = $this->config->settings; @@ -156,6 +155,8 @@ class CreateController extends AbstractController $this->addScriptFiles(['vendor/jquery/jquery.uitablefilter.js', 'indexes.js']); + $this->checkParameters(['server', 'db', 'table', 'num_fields']); + $templateData = $this->columnsDefinition->displayForm('/table/create', $GLOBALS['num_fields']); $this->render('columns_definitions/column_definitions_form', $templateData); diff --git a/libraries/classes/Controllers/Table/DeleteConfirmController.php b/libraries/classes/Controllers/Table/DeleteConfirmController.php index da751966b4..c5ee9cfe5d 100644 --- a/libraries/classes/Controllers/Table/DeleteConfirmController.php +++ b/libraries/classes/Controllers/Table/DeleteConfirmController.php @@ -26,7 +26,7 @@ final class DeleteConfirmController extends AbstractController return; } - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Controllers/Table/DropColumnConfirmationController.php b/libraries/classes/Controllers/Table/DropColumnConfirmationController.php index 8309fa71df..e2c795e8eb 100644 --- a/libraries/classes/Controllers/Table/DropColumnConfirmationController.php +++ b/libraries/classes/Controllers/Table/DropColumnConfirmationController.php @@ -24,7 +24,7 @@ final class DropColumnConfirmationController extends AbstractController return; } - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Controllers/Table/ExportController.php b/libraries/classes/Controllers/Table/ExportController.php index 81cb1224b1..8ab6d99bb5 100644 --- a/libraries/classes/Controllers/Table/ExportController.php +++ b/libraries/classes/Controllers/Table/ExportController.php @@ -44,7 +44,7 @@ class ExportController extends AbstractController $this->addScriptFiles(['export.js']); - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Controllers/Table/FindReplaceController.php b/libraries/classes/Controllers/Table/FindReplaceController.php index c49e908710..3029f235bc 100644 --- a/libraries/classes/Controllers/Table/FindReplaceController.php +++ b/libraries/classes/Controllers/Table/FindReplaceController.php @@ -60,7 +60,7 @@ class FindReplaceController extends AbstractController public function __invoke(): void { - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Controllers/Table/GetFieldController.php b/libraries/classes/Controllers/Table/GetFieldController.php index c033e18f52..a481bf03cd 100644 --- a/libraries/classes/Controllers/Table/GetFieldController.php +++ b/libraries/classes/Controllers/Table/GetFieldController.php @@ -40,11 +40,7 @@ class GetFieldController extends AbstractController { $this->response->disable(); - /* Check parameters */ - Util::checkParameters([ - 'db', - 'table', - ]); + $this->checkParameters(['db', 'table']); /* Select database */ if (! $this->dbi->selectDb($GLOBALS['db'])) { diff --git a/libraries/classes/Controllers/Table/GisVisualizationController.php b/libraries/classes/Controllers/Table/GisVisualizationController.php index 82ea067946..cfd37f3d40 100644 --- a/libraries/classes/Controllers/Table/GisVisualizationController.php +++ b/libraries/classes/Controllers/Table/GisVisualizationController.php @@ -41,7 +41,7 @@ final class GisVisualizationController extends AbstractController public function __invoke(): void { - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Table/ImportController.php b/libraries/classes/Controllers/Table/ImportController.php index e156b3052e..4a2098eab3 100644 --- a/libraries/classes/Controllers/Table/ImportController.php +++ b/libraries/classes/Controllers/Table/ImportController.php @@ -46,7 +46,7 @@ final class ImportController extends AbstractController $this->addScriptFiles(['import.js']); - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Controllers/Table/IndexRenameController.php b/libraries/classes/Controllers/Table/IndexRenameController.php index 8965ef6a56..38a74d96f0 100644 --- a/libraries/classes/Controllers/Table/IndexRenameController.php +++ b/libraries/classes/Controllers/Table/IndexRenameController.php @@ -38,7 +38,7 @@ final class IndexRenameController extends AbstractController public function __invoke(): void { if (! isset($_POST['create_edit_table'])) { - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Controllers/Table/IndexesController.php b/libraries/classes/Controllers/Table/IndexesController.php index c2dd4f2396..1ae9e531f3 100644 --- a/libraries/classes/Controllers/Table/IndexesController.php +++ b/libraries/classes/Controllers/Table/IndexesController.php @@ -45,7 +45,7 @@ class IndexesController extends AbstractController public function __invoke(): void { if (! isset($_POST['create_edit_table'])) { - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Controllers/Table/OperationsController.php b/libraries/classes/Controllers/Table/OperationsController.php index 509be58036..cf2539e502 100644 --- a/libraries/classes/Controllers/Table/OperationsController.php +++ b/libraries/classes/Controllers/Table/OperationsController.php @@ -77,7 +77,7 @@ class OperationsController extends AbstractController $this->addScriptFiles(['table/operations.js']); - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $isSystemSchema = Utilities::isSystemSchema($GLOBALS['db']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; diff --git a/libraries/classes/Controllers/Table/ReplaceController.php b/libraries/classes/Controllers/Table/ReplaceController.php index 07f0fed202..0f252fa804 100644 --- a/libraries/classes/Controllers/Table/ReplaceController.php +++ b/libraries/classes/Controllers/Table/ReplaceController.php @@ -69,7 +69,7 @@ final class ReplaceController extends AbstractController public function __invoke(): void { - Util::checkParameters(['db', 'table', 'goto']); + $this->checkParameters(['db', 'table', 'goto']); $this->dbi->selectDb($GLOBALS['db']); diff --git a/libraries/classes/Controllers/Table/SearchController.php b/libraries/classes/Controllers/Table/SearchController.php index 78c2600072..3687b1756f 100644 --- a/libraries/classes/Controllers/Table/SearchController.php +++ b/libraries/classes/Controllers/Table/SearchController.php @@ -171,7 +171,7 @@ class SearchController extends AbstractController */ public function __invoke(): void { - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Controllers/Table/SqlController.php b/libraries/classes/Controllers/Table/SqlController.php index 6affce9777..8f6b98f031 100644 --- a/libraries/classes/Controllers/Table/SqlController.php +++ b/libraries/classes/Controllers/Table/SqlController.php @@ -45,7 +45,7 @@ final class SqlController extends AbstractController $this->response->addHTML($pageSettings->getErrorHTML()); $this->response->addHTML($pageSettings->getHTML()); - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $url_params = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Controllers/Table/Structure/ChangeController.php b/libraries/classes/Controllers/Table/Structure/ChangeController.php index 5dfc76566a..c70a2e5235 100644 --- a/libraries/classes/Controllers/Table/Structure/ChangeController.php +++ b/libraries/classes/Controllers/Table/Structure/ChangeController.php @@ -95,6 +95,8 @@ final class ChangeController extends AbstractController $this->addScriptFiles(['vendor/jquery/jquery.uitablefilter.js', 'indexes.js']); + $this->checkParameters(['server', 'db', 'table', 'num_fields']); + $templateData = $this->columnsDefinition->displayForm( '/table/structure/save', $GLOBALS['num_fields'], diff --git a/libraries/classes/Controllers/Table/Structure/PrimaryController.php b/libraries/classes/Controllers/Table/Structure/PrimaryController.php index a7a6fa5051..7ce9333069 100644 --- a/libraries/classes/Controllers/Table/Structure/PrimaryController.php +++ b/libraries/classes/Controllers/Table/Structure/PrimaryController.php @@ -58,7 +58,7 @@ final class PrimaryController extends AbstractController $mult_btn = $_POST['mult_btn'] ?? $mult_btn ?? ''; if (! empty($selected_fld) && ! empty($primary)) { - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Controllers/Table/StructureController.php b/libraries/classes/Controllers/Table/StructureController.php index fd99cb9fad..6fe98a43fb 100644 --- a/libraries/classes/Controllers/Table/StructureController.php +++ b/libraries/classes/Controllers/Table/StructureController.php @@ -117,7 +117,7 @@ class StructureController extends AbstractController $relationParameters = $this->relation->getRelationParameters(); - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $isSystemSchema = Utilities::isSystemSchema($GLOBALS['db']); $url_params = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; diff --git a/libraries/classes/Controllers/Table/TrackingController.php b/libraries/classes/Controllers/Table/TrackingController.php index 62ce85815b..b972bb26de 100644 --- a/libraries/classes/Controllers/Table/TrackingController.php +++ b/libraries/classes/Controllers/Table/TrackingController.php @@ -42,7 +42,7 @@ final class TrackingController extends AbstractController define('TABLE_MAY_BE_ABSENT', true); - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Controllers/Table/TriggersController.php b/libraries/classes/Controllers/Table/TriggersController.php index 2a8e859281..773c1aad63 100644 --- a/libraries/classes/Controllers/Table/TriggersController.php +++ b/libraries/classes/Controllers/Table/TriggersController.php @@ -42,7 +42,7 @@ class TriggersController extends AbstractController * Displays the header and tabs */ if (! empty($GLOBALS['table']) && in_array($GLOBALS['table'], $this->dbi->getTables($GLOBALS['db']))) { - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); @@ -52,7 +52,7 @@ class TriggersController extends AbstractController } else { $GLOBALS['table'] = ''; - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/Table/ZoomSearchController.php b/libraries/classes/Controllers/Table/ZoomSearchController.php index a5601a9935..d562167562 100644 --- a/libraries/classes/Controllers/Table/ZoomSearchController.php +++ b/libraries/classes/Controllers/Table/ZoomSearchController.php @@ -94,7 +94,7 @@ class ZoomSearchController extends AbstractController public function __invoke(): void { - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Controllers/View/CreateController.php b/libraries/classes/Controllers/View/CreateController.php index 023a5840eb..b7a755a758 100644 --- a/libraries/classes/Controllers/View/CreateController.php +++ b/libraries/classes/Controllers/View/CreateController.php @@ -44,7 +44,7 @@ class CreateController extends AbstractController public function __invoke(): void { - Util::checkParameters(['db']); + $this->checkParameters(['db']); $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabDatabase'], 'database'); $GLOBALS['errorUrl'] .= Url::getCommon(['db' => $GLOBALS['db']], '&'); diff --git a/libraries/classes/Controllers/View/OperationsController.php b/libraries/classes/Controllers/View/OperationsController.php index e80eb24d79..644cf89478 100644 --- a/libraries/classes/Controllers/View/OperationsController.php +++ b/libraries/classes/Controllers/View/OperationsController.php @@ -45,7 +45,7 @@ class OperationsController extends AbstractController $this->addScriptFiles(['table/operations.js']); - Util::checkParameters(['db', 'table']); + $this->checkParameters(['db', 'table']); $GLOBALS['urlParams'] = ['db' => $GLOBALS['db'], 'table' => $GLOBALS['table']]; $GLOBALS['errorUrl'] = Util::getScriptNameForOption($GLOBALS['cfg']['DefaultTabTable'], 'table'); diff --git a/libraries/classes/Table/ColumnsDefinition.php b/libraries/classes/Table/ColumnsDefinition.php index ca9002fd89..535af8a894 100644 --- a/libraries/classes/Table/ColumnsDefinition.php +++ b/libraries/classes/Table/ColumnsDefinition.php @@ -68,13 +68,6 @@ final class ColumnsDefinition ?array $selected = null, $fields_meta = null ): array { - Util::checkParameters([ - 'server', - 'db', - 'table', - 'num_fields', - ]); - $length_values_input_size = 8; $content_cells = []; $form_params = ['db' => $GLOBALS['db']]; diff --git a/libraries/classes/Util.php b/libraries/classes/Util.php index 12f84ca740..dbca164a07 100644 --- a/libraries/classes/Util.php +++ b/libraries/classes/Util.php @@ -6,7 +6,6 @@ namespace PhpMyAdmin; use PhpMyAdmin\Dbal\ResultInterface; use PhpMyAdmin\Html\Generator; -use PhpMyAdmin\Html\MySQLDocumentation; use PhpMyAdmin\Query\Utilities; use PhpMyAdmin\SqlParser\Components\Expression; use PhpMyAdmin\SqlParser\Context; @@ -22,7 +21,6 @@ use function array_map; use function array_merge; use function array_shift; use function array_unique; -use function basename; use function bin2hex; use function chr; use function count; @@ -800,46 +798,6 @@ class Util ); } - /** - * Function added to avoid path disclosures. - * Called by each script that needs parameters, it displays - * an error message and, by default, stops the execution. - * - * @param string[] $params The names of the parameters needed by the calling - * script - * @param bool $request Check parameters in request - */ - public static function checkParameters($params, $request = false): void - { - $reportedScriptName = basename($GLOBALS['PMA_PHP_SELF']); - $foundError = false; - $errorMessage = ''; - if ($request) { - $array = $_REQUEST; - } else { - $array = $GLOBALS; - } - - foreach ($params as $param) { - if (isset($array[$param])) { - continue; - } - - $errorMessage .= $reportedScriptName - . ': ' . __('Missing parameter:') . ' ' - . $param - . MySQLDocumentation::showDocumentation('faq', 'faqmissingparameters', true) - . '[br]'; - $foundError = true; - } - - if (! $foundError) { - return; - } - - Core::fatalError($errorMessage); - } - /** * Build a condition and with a value * diff --git a/test/classes/Controllers/AbstractControllerTest.php b/test/classes/Controllers/AbstractControllerTest.php new file mode 100644 index 0000000000..b56b9822bb --- /dev/null +++ b/test/classes/Controllers/AbstractControllerTest.php @@ -0,0 +1,83 @@ + $params + */ + public function testCheckParameters(array $params): void + { + parent::checkParameters($params); + } + }; + + \PhpMyAdmin\ResponseRenderer::getInstance()->setAjax(false); + + $GLOBALS['param1'] = 'param1'; + $GLOBALS['param2'] = null; + + $message = 'index.php: Missing parameter: param2'; + $message .= MySQLDocumentation::showDocumentation('faq', 'faqmissingparameters', true); + $message .= '[br]'; + $expected = $template->render('error/generic', [ + 'lang' => 'en', + 'dir' => 'ltr', + 'error_message' => Sanitize::sanitizeMessage($message), + ]); + + $this->expectOutputString($expected); + + $controller->testCheckParameters(['param1', 'param2']); + + $this->assertSame(400, $response->getHttpResponseCode()); + } + + public function testCheckParametersWithAllParameters(): void + { + $_REQUEST = []; + + $response = new ResponseRenderer(); + $template = new Template(); + $controller = new class ($response, $template) extends AbstractController { + /** + * @psalm-param non-empty-list $params + */ + public function testCheckParameters(array $params): void + { + parent::checkParameters($params); + } + }; + + \PhpMyAdmin\ResponseRenderer::getInstance()->setAjax(false); + + $GLOBALS['param1'] = 'param1'; + $GLOBALS['param2'] = 'param2'; + + $this->expectOutputString(''); + + $controller->testCheckParameters(['param1', 'param2']); + + $this->assertSame(200, $response->getHttpResponseCode()); + } +} diff --git a/test/classes/UtilTest.php b/test/classes/UtilTest.php index beedb613e6..7ca5bb9d0b 100644 --- a/test/classes/UtilTest.php +++ b/test/classes/UtilTest.php @@ -4,11 +4,9 @@ declare(strict_types=1); namespace PhpMyAdmin\Tests; -use PhpMyAdmin\Core; use PhpMyAdmin\DatabaseInterface; use PhpMyAdmin\FieldMetadata; use PhpMyAdmin\MoTranslator\Loader; -use PhpMyAdmin\ResponseRenderer; use PhpMyAdmin\SqlParser\Context; use PhpMyAdmin\SqlParser\Token; use PhpMyAdmin\Util; @@ -334,52 +332,6 @@ class UtilTest extends AbstractTestCase $this->assertArrayNotHasKey('is_superuser', $_SESSION['cache']['server_server']); } - public function testCheckParameterMissing(): void - { - parent::setGlobalConfig(); - $_REQUEST = []; - $GLOBALS['text_dir'] = 'ltr'; - $GLOBALS['PMA_PHP_SELF'] = Core::getenv('PHP_SELF'); - $GLOBALS['db'] = 'db'; - $GLOBALS['table'] = 'table'; - $GLOBALS['server'] = 1; - $GLOBALS['cfg']['ServerDefault'] = 1; - $GLOBALS['cfg']['AllowThirdPartyFraming'] = false; - ResponseRenderer::getInstance()->setAjax(false); - - $this->expectOutputRegex('/Missing parameter: field/'); - - Util::checkParameters( - [ - 'db', - 'table', - 'field', - ] - ); - } - - public function testCheckParameter(): void - { - parent::setGlobalConfig(); - $GLOBALS['cfg'] = ['ServerDefault' => 1]; - $GLOBALS['text_dir'] = 'ltr'; - $GLOBALS['PMA_PHP_SELF'] = Core::getenv('PHP_SELF'); - $GLOBALS['db'] = 'dbDatabase'; - $GLOBALS['table'] = 'tblTable'; - $GLOBALS['field'] = 'test_field'; - $GLOBALS['sql_query'] = 'SELECT * FROM tblTable;'; - - $this->expectOutputString(''); - Util::checkParameters( - [ - 'db', - 'table', - 'field', - 'sql_query', - ] - ); - } - /** * Test for Util::convertBitDefaultValue *