Merge pull request from GHSA-2xv9-886q-p7xx

Fix that custom variable protection and blacklists can be circumvented

modules/monitoring/application/controllers/ListController.php

@@ -4,6 +4,7 @@
 namespace Icinga\Module\Monitoring\Controllers;
 
 use Icinga\Security\SecurityException;
+use Icinga\Util\GlobFilter;
 use Icinga\Web\Form;
 use Zend_Form;
 use Icinga\Data\Filter\Filter;
@@ -761,8 +762,31 @@ class ListController extends Controller
             -1,
             PREG_SPLIT_NO_EMPTY
         );
-        $this->view->addColumns = $columns;
-        return $columns;
+
+        $customVars = [];
+        $additionalCols = [];
+        foreach ($columns as $column) {
+            if (preg_match('~^_(host|service)_([a-zA-Z0-9_]+)$~', $column, $m)) {
+                $customVars[$m[1]]['vars'][$m[2]] = null;
+            } else {
+                $additionalCols[] = $column;
+            }
+        }
+
+        if (! empty($customVars)) {
+            $blacklistedProperties = new GlobFilter(
+                $this->getRestrictions('monitoring/blacklist/properties')
+            );
+            $customVars = $blacklistedProperties->removeMatching($customVars);
+            foreach ($customVars as $type => $vars) {
+                foreach ($vars['vars'] as $var => $_) {
+                    $additionalCols[] = '_' . $type . '_' . $var;
+                }
+            }
+        }
+
+        $this->view->addColumns = $additionalCols;
+        return $additionalCols;
     }
 
     protected function addTitleTab($action, $title, $tip)

modules/monitoring/application/views/scripts/list/hosts.phtml

@@ -92,7 +92,11 @@ if (! $this->compact): ?>
                 <p class="overview-plugin-output"><?= $this->pluginOutput($this->ellipsis($host->host_output, 10000), true, $host->host_check_command) ?></p>
             </td>
         <?php foreach($this->addColumns as $col): ?>
+          <?php if ($host->$col && preg_match('~^_(host|service)_([a-zA-Z0-9_]+)$~', $col, $m)): ?>
+            <td><?= $this->escape(\Icinga\Module\Monitoring\Object\MonitoredObject::protectCustomVars([$m[2] => $host->$col])[$m[2]]) ?></td>
+          <?php else: ?>
             <td><?= $this->escape($host->$col) ?></td>
+          <?php endif ?>
         <?php endforeach ?>
         </tr>
     <?php endforeach ?>

modules/monitoring/application/views/scripts/list/services.phtml

@@ -120,7 +120,11 @@ if (! $this->compact): ?>
                 </div>
             </td>
         <?php foreach($this->addColumns as $col): ?>
+          <?php if ($service->$col && preg_match('~^_(host|service)_([a-zA-Z0-9_]+)$~', $col, $m)): ?>
+            <td><?= $this->escape(\Icinga\Module\Monitoring\Object\MonitoredObject::protectCustomVars([$m[2] => $service->$col])[$m[2]]) ?></td>
+          <?php else: ?>
             <td><?= $this->escape($service->$col) ?></td>
+          <?php endif ?>
         <?php endforeach ?>
         </tr>
     <?php endforeach ?>

modules/monitoring/library/Monitoring/Controller.php

@@ -3,11 +3,13 @@
 
 namespace Icinga\Module\Monitoring;
 
+use ArrayIterator;
 use Icinga\Exception\ConfigurationError;
 use Icinga\Exception\QueryException;
 use Icinga\Data\Filter\Filter;
 use Icinga\Data\Filterable;
 use Icinga\File\Csv;
+use Icinga\Module\Monitoring\Data\CustomvarProtectionIterator;
 use Icinga\Util\Json;
 use Icinga\Web\Controller as IcingaWebController;
 use Icinga\Web\Url;
@@ -60,7 +62,15 @@ class Controller extends IcingaWebController
                         'Content-Disposition',
                         'inline; filename=' . $this->getRequest()->getActionName() . '.json'
                     )
-                    ->appendBody(Json::sanitize($query->fetchAll()))
+                    ->appendBody(
+                        Json::sanitize(
+                            iterator_to_array(
+                                new CustomvarProtectionIterator(
+                                    new ArrayIterator($query->fetchAll())
+                                )
+                            )
+                        )
+                    )
                     ->sendResponse();
                 exit;
             case 'csv':
@@ -72,7 +82,7 @@ class Controller extends IcingaWebController
                         'Content-Disposition',
                         'attachment; filename=' . $this->getRequest()->getActionName() . '.csv'
                     )
-                    ->appendBody((string) Csv::fromQuery($query))
+                    ->appendBody((string) Csv::fromQuery(new CustomvarProtectionIterator($query)))
                     ->sendResponse();
                 exit;
         }

modules/monitoring/library/Monitoring/Data/CustomvarProtectionIterator.php

@@ -0,0 +1,25 @@
+<?php
+/* Icinga Web 2 | (c) 2021 Icinga GmbH | GPLv2+ */
+
+namespace Icinga\Module\Monitoring\Data;
+
+use Icinga\Module\Monitoring\Object\MonitoredObject;
+use IteratorIterator;
+
+class CustomvarProtectionIterator extends IteratorIterator
+{
+    const IS_CV_RE = '~^_(host|service)_([a-zA-Z0-9_]+)$~';
+
+    public function current()
+    {
+        $row = parent::current();
+
+        foreach ($row as $col => $val) {
+            if (preg_match(self::IS_CV_RE, $col, $m)) {
+                $row->$col = MonitoredObject::protectCustomVars([$m[2] => $val])[$m[2]];
+            }
+        }
+
+        return $row;
+    }
+}

modules/monitoring/library/Monitoring/Object/MonitoredObject.php

@@ -418,7 +418,37 @@ abstract class MonitoredObject implements Filterable
      */
     public function fetchCustomvars()
     {
-        $blacklist = array();
+
+        if ($this->type === self::TYPE_SERVICE) {
+            $this->fetchServiceVariables();
+            $customvars = $this->serviceVariables;
+        } else {
+            $this->fetchHostVariables();
+            $customvars = $this->hostVariables;
+        }
+
+        $this->customvars = $customvars;
+        $this->hideBlacklistedProperties();
+        $this->customvars = $this->obfuscateCustomVars($this->customvars, null);
+
+        return $this;
+    }
+
+    /**
+     * Obfuscate custom variables recursively
+     *
+     * @param stdClass|array    $customvars         The custom variables to obfuscate
+     *
+     * @return stdClass|array                       The obfuscated custom variables
+     */
+    protected function obfuscateCustomVars($customvars, $_)
+    {
+        return self::protectCustomVars($customvars);
+    }
+
+    public static function protectCustomVars($customvars)
+    {
+        $blacklist = [];
         $blacklistPattern = '';
 
         if (($blacklistConfig = Config::module('monitoring')->get('security', 'protected_customvars', '')) !== '') {
@@ -432,44 +462,24 @@ abstract class MonitoredObject implements Filterable
             $blacklistPattern = '/^(' . implode('|', $blacklist) . ')$/i';
         }
 
-        if ($this->type === self::TYPE_SERVICE) {
-            $this->fetchServiceVariables();
-            $customvars = $this->serviceVariables;
-        } else {
-            $this->fetchHostVariables();
-            $customvars = $this->hostVariables;
+        if (! $blacklistPattern) {
+            return $customvars;
         }
 
-        $this->customvars = $customvars;
-        $this->hideBlacklistedProperties();
-
-        if ($blacklistPattern) {
-            $this->customvars = $this->obfuscateCustomVars($this->customvars, $blacklistPattern);
-        }
-
-        return $this;
-    }
-
-    /**
-     * Obfuscate custom variables recursively
-     *
-     * @param stdClass|array    $customvars         The custom variables to obfuscate
-     * @param string            $blacklistPattern   Which custom variables to obfuscate
-     *
-     * @return stdClass|array                       The obfuscated custom variables
-     */
-    protected function obfuscateCustomVars($customvars, $blacklistPattern)
-    {
-        $obfuscatedCustomVars = array();
-        foreach ($customvars as $name => $value) {
-            if ($blacklistPattern && preg_match($blacklistPattern, $name)) {
-                $obfuscatedCustomVars[$name] = '***';
-            } else {
-                $obfuscatedCustomVars[$name] = $value instanceof stdClass || is_array($value)
-                    ? $this->obfuscateCustomVars($value, $blacklistPattern)
-                    : $value;
+        $obfuscatedCustomVars = [];
+        $obfuscator = function ($vars) use ($blacklistPattern, &$obfuscatedCustomVars, &$obfuscator) {
+            foreach ($vars as $name => $value) {
+                if ($blacklistPattern && preg_match($blacklistPattern, $name)) {
+                    $obfuscatedCustomVars[$name] = '***';
+                } else {
+                    $obfuscatedCustomVars[$name] = $value instanceof stdClass || is_array($value)
+                        ? $obfuscator($value)
+                        : $value;
+                }
             }
-        }
+        };
+        $obfuscator($customvars);
+
         return $customvars instanceof stdClass ? (object) $obfuscatedCustomVars : $obfuscatedCustomVars;
     }