From 8d84954143e77220a7fe2ae27963525005c60793 Mon Sep 17 00:00:00 2001 From: Sandra Kuipers Date: Mon, 24 Jan 2022 10:35:36 +0800 Subject: [PATCH] System: improve sanitization of page actions --- export.php | 27 +++++++++++++---------- fullscreen.php | 2 +- gibbon.php | 10 ++++----- report.php | 32 ++++++++++++++-------------- resources/templates/report.twig.html | 7 ++++++ src/View/Page.php | 19 +++++++++++------ 6 files changed, 57 insertions(+), 40 deletions(-) diff --git a/export.php b/export.php index 7a0b0c0f39..5833539ea1 100755 --- a/export.php +++ b/export.php @@ -18,7 +18,10 @@ */ //Gibbon system-wide includes -include './gibbon.php'; +require_once './gibbon.php'; + +// Setup the Page and Session objects +$page = $container->get('page'); $session->set('sidebarExtra', ''); //Check to see if system settings are set from databases @@ -31,18 +34,20 @@ exit; } -$session->set('address', $_GET['q'] ?? ''); -$session->set('module', getModuleName($session->get('address'))); -$session->set('action', getActionName($session->get('address'))); +$address = $page->getAddress(); -if (empty($session->get('address')) || strstr($session->get('address'), '..') != false) { +if (empty($address) || $page->isAddressValid($address, true) == false || stripos($address, 'modules') === false) { header("HTTP/1.1 403 Forbidden"); exit; +} + +$session->set('address', $address); +$session->set('module', getModuleName($address)); +$session->set('action', getActionName($address)); + +if (is_file('./'.$address)) { + include './'.$address; } else { - if (is_file('./'.$session->get('address'))) { - include './'.$session->get('address'); - } else { - header("HTTP/1.1 404 Not Found"); - exit; - } + header("HTTP/1.1 404 Not Found"); + exit; } diff --git a/fullscreen.php b/fullscreen.php index 409575634d..f54e892fc6 100644 --- a/fullscreen.php +++ b/fullscreen.php @@ -42,7 +42,7 @@ if (empty($address)) { $page->addWarning(__('There is no content to display')); -} elseif ($page->isAddressValid($address) == false) { +} elseif ($page->isAddressValid($address, true) == false) { $page->addError(__('Illegal address detected: access denied.')); } else { // Pass these globals into the script of the included file, for backwards compatibility. diff --git a/gibbon.php b/gibbon.php index f8e3cabeea..5f6517180c 100644 --- a/gibbon.php +++ b/gibbon.php @@ -102,8 +102,8 @@ } // Globals for backwards compatibility -$gibbon->session = $container->get('session'); $session = $container->get('session'); +$gibbon->session = $session; $container->share(\Gibbon\Contracts\Services\Session::class, $session); // Setup global absoluteURL for all urls. @@ -126,11 +126,11 @@ } // Autoload the current module namespace -if (!empty($gibbon->session->get('module'))) { - $moduleNamespace = preg_replace('/[^a-zA-Z0-9]/', '', $gibbon->session->get('module')); - $autoloader->addPsr4('Gibbon\\Module\\'.$moduleNamespace.'\\', realpath(__DIR__).'/modules/'.$gibbon->session->get('module').'/src'); +if (!empty($session->get('module'))) { + $moduleNamespace = preg_replace('/[^a-zA-Z0-9]/', '', $session->get('module')); + $autoloader->addPsr4('Gibbon\\Module\\'.$moduleNamespace.'\\', realpath(__DIR__).'/modules/'.$session->get('module').'/src'); // Temporary backwards-compatibility for external modules (Query Builder) - $autoloader->addPsr4('Gibbon\\'.$moduleNamespace.'\\', realpath(__DIR__).'/modules/'.$gibbon->session->get('module')); + $autoloader->addPsr4('Gibbon\\'.$moduleNamespace.'\\', realpath(__DIR__).'/modules/'.$session->get('module')); $autoloader->register(true); } diff --git a/report.php b/report.php index 48b1486868..fa6547c546 100644 --- a/report.php +++ b/report.php @@ -40,7 +40,7 @@ if (empty($address)) { $page->addWarning(__('There is no content to display')); -} elseif ($page->isAddressValid($address) == false) { +} elseif ($page->isAddressValid($address, true) == false || stripos($address, 'modules') === false) { $page->addError(__('Illegal address detected: access denied.')); } else { // Pass these globals into the script of the included file, for backwards compatibility. @@ -59,6 +59,21 @@ if (is_file('./'.$address)) { $page->writeFromFile('./'.$address, $globals); + + $page->addData([ + 'isLoggedIn' => $session->has('username') && $session->has('gibbonRoleIDCurrent'), + 'username' => $session->get('username'), + 'gibbonThemeName' => $session->get('gibbonThemeName'), + 'organisationName' => $session->get('organisationName'), + 'organisationNameShort' => $session->get('organisationNameShort'), + 'organisationAdministratorName' => $session->get('organisationAdministratorName'), + 'organisationAdministratorEmail' => $session->get('organisationAdministratorEmail'), + 'organisationLogo' => $session->get('organisationLogo'), + 'time' => Format::time(date('H:i:s')), + 'date' => Format::date(date('Y-m-d')), + 'rightToLeft' => $session->get('i18n')['rtl'] == 'Y', + 'orientation' => $_GET['orientation'] ?? 'P', + ]); } else { $page->writeFromTemplate('error.twig.html'); } @@ -68,19 +83,4 @@ $page->stylesheets->add('theme-dev', 'resources/assets/css/theme.min.css'); $page->stylesheets->add('core', 'resources/assets/css/core.min.css', ['weight' => 10]); -$page->addData([ - 'isLoggedIn' => $session->has('username') && $session->has('gibbonRoleIDCurrent'), - 'username' => $session->get('username'), - 'gibbonThemeName' => $session->get('gibbonThemeName'), - 'organisationName' => $session->get('organisationName'), - 'organisationNameShort' => $session->get('organisationNameShort'), - 'organisationAdministratorName' => $session->get('organisationAdministratorName'), - 'organisationAdministratorEmail' => $session->get('organisationAdministratorEmail'), - 'organisationLogo' => $session->get('organisationLogo'), - 'time' => Format::time(date('H:i:s')), - 'date' => Format::date(date('Y-m-d')), - 'rightToLeft' => $session->get('i18n')['rtl'] == 'Y', - 'orientation' => $_GET['orientation'] ?? 'P', -]); - echo $page->render('report.twig.html'); diff --git a/resources/templates/report.twig.html b/resources/templates/report.twig.html index 76ef439c21..ba0c2ec42a 100644 --- a/resources/templates/report.twig.html +++ b/resources/templates/report.twig.html @@ -29,6 +29,13 @@
+ + {% for type, alerts in page.alerts %} + {% for text in alerts %} +
{{ text|raw }}
+ {% endfor %} + {% endfor %} + {% if isLoggedIn %} {{ content|join("\n")|raw }} {% endif %} diff --git a/src/View/Page.php b/src/View/Page.php index d06a5a87c6..147556a9b4 100644 --- a/src/View/Page.php +++ b/src/View/Page.php @@ -174,7 +174,7 @@ public function getTitle(): string */ public function getAddress(): string { - return $this->address; + return preg_replace('/[^a-zA-Z0-9_\-\.\/\s&=%]/', '', $this->address); } /** @@ -424,14 +424,19 @@ public function setDefaults($absolutePath) * @param string $address * @return bool */ - public function isAddressValid($address) : bool + public function isAddressValid($address, bool $strictPHP = false) : bool { + if ($strictPHP && stripos($address, '.php') === false) { + return false; + } + return !(stripos($address, '..') !== false - || strstr($address, 'installer') - || strstr($address, 'uploads') - || in_array($address, array('index.php', '/index.php', './index.php')) - || substr($address, -11) == '// index.php' - || substr($address, -11) == './index.php'); + || stristr($address, 'installer') + || stristr($address, 'uploads') + || stristr($address, 'config.php') + || in_array(strtolower($address), array('index.php', '/index.php', './index.php')) + || strtolower(substr($address, -11)) == '// index.php' + || strtolower(substr($address, -11)) == './index.php'); } /**