System: improve sanitization of page actions

GibbonEdu · Jan 24, 2022 · 8d84954 · 8d84954
1 parent 4a735c6
commit 8d84954
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 40 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -29,6 +29,13 @@
             </div>
 
             <div id="content-wrap-report" class="w-full max-w-full">
+
+                {% for type, alerts in page.alerts %}
+                    {% for text in alerts %}
+                        <div class="{{ type }}">{{ text|raw }}</div>
+                    {% endfor %}
+                {% endfor %}
+
                 {% if isLoggedIn %}
                     {{ content|join("\n")|raw }}
                 {% endif %}

diff --git 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');
     }
 
     /**