From 8d6c18db7e370dab2a8a2a2df2c57ef5fad3358c Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 28 Sep 2026 19:32:25 -0400 Subject: [PATCH] fix: only dispatch set()/changes() keys to field accessors refs GHSA-vvx7-ghpv-jq98 ZM_Object::set() called any method whose name matched a key in its data, and changes() called it as a getter. That data is usually a request array (filter[...], newMonitor[...], user[...]), so a request could invoke save(), delete(), execute() and the like. filterdebug with fid=0 did exactly that before its authorization check: filter[save][...] stored an AutoExecute filter with a chosen command and filter[execute] ran zmfilter.pl on it, giving command execution to any logged-in user. The filter and events views pass filter[...] to set() the same way. set() and changes() now dispatch a key to a method only when the key is a field in $defaults or is listed in the class's new static $setters, the accessors outside $defaults that take a value (Filter's query accessors, Monitor::Model/Manufacturer/Groups, User::Role, and so on). Other method names are refused with a warning. filterdebug also requires Events view before it builds a filter from the request. Co-Authored-By: Claude Opus 5.5 --- web/ajax/modals/filterdebug.php | 6 ++++++ web/includes/Event.php | 1 + web/includes/Filter.php | 1 + web/includes/Group.php | 1 + web/includes/Group_Permission.php | 1 + web/includes/Monitor.php | 1 + web/includes/Monitor_Permission.php | 1 + web/includes/Object.php | 18 ++++++++++++++++++ web/includes/Role_Group_Permission.php | 1 + web/includes/Role_Monitor_Permission.php | 1 + web/includes/User.php | 1 + web/includes/User_Role.php | 1 + 12 files changed, 34 insertions(+) diff --git a/web/ajax/modals/filterdebug.php b/web/ajax/modals/filterdebug.php index 62b216b95..27831e337 100644 --- a/web/ajax/modals/filterdebug.php +++ b/web/ajax/modals/filterdebug.php @@ -7,6 +7,12 @@ // any authenticated user. refs GHSA-28mv-hqxw-qw84 $fid = validInt($_REQUEST['fid']); + // Authorize before applying any request input to the filter. refs GHSA-vvx7-ghpv-jq98 + if (!canView('Events')) { + $view = 'error'; + return; + } + $filter = null; if ($fid) { $filter = new ZM\Filter($fid); diff --git a/web/includes/Event.php b/web/includes/Event.php index 1c963d6f4..414f00991 100644 --- a/web/includes/Event.php +++ b/web/includes/Event.php @@ -7,6 +7,7 @@ require_once('Event_Tag.php'); require_once('Tag.php'); class Event extends ZM_Object { + protected static $setters = array('Storage', 'SecondaryStorage'); protected static $table = 'Events'; protected $Tags; diff --git a/web/includes/Filter.php b/web/includes/Filter.php index 7ad961f25..7065fc3f1 100644 --- a/web/includes/Filter.php +++ b/web/includes/Filter.php @@ -5,6 +5,7 @@ require_once('FilterTerm.php'); require_once('Monitor.php'); class Filter extends ZM_Object { + protected static $setters = array('Query_json', 'Query', 'terms', 'sort_field', 'sort_asc', 'skip_locked', 'limit'); protected static $table = 'Filters'; protected static $attrTypes = null; protected static $opTypes = null; diff --git a/web/includes/Group.php b/web/includes/Group.php index ae39a119b..a3a91cfb0 100644 --- a/web/includes/Group.php +++ b/web/includes/Group.php @@ -2,6 +2,7 @@ namespace ZM; class Group extends ZM_Object { + protected static $setters = array('depth', 'Permissions'); protected static $table = 'Groups'; protected static $permissions = array(); protected $defaults = array( diff --git a/web/includes/Group_Permission.php b/web/includes/Group_Permission.php index fd45fadbe..e9d6cd9b3 100644 --- a/web/includes/Group_Permission.php +++ b/web/includes/Group_Permission.php @@ -8,6 +8,7 @@ require_once('User.php'); require_once('Group.php'); class Group_Permission extends ZM_Object { + protected static $setters = array('Group', 'User'); protected static $table = 'Groups_Permissions'; protected $defaults = array( 'Id' => null, diff --git a/web/includes/Monitor.php b/web/includes/Monitor.php index c3731f163..56950ad7f 100644 --- a/web/includes/Monitor.php +++ b/web/includes/Monitor.php @@ -11,6 +11,7 @@ require_once('Storage.php'); require_once('Zone.php'); class Monitor extends ZM_Object { + protected static $setters = array('User', 'ViewWidth', 'ViewHeight', 'Storage', 'Groups', 'connKey', 'Model', 'Manufacturer'); private $shm_id = null; private $connected = false; diff --git a/web/includes/Monitor_Permission.php b/web/includes/Monitor_Permission.php index e36cefd8e..e71afbb6c 100644 --- a/web/includes/Monitor_Permission.php +++ b/web/includes/Monitor_Permission.php @@ -8,6 +8,7 @@ require_once('User.php'); require_once('Monitor.php'); class Monitor_Permission extends ZM_Object { + protected static $setters = array('Monitor', 'User'); protected static $table = 'Monitors_Permissions'; protected $defaults = array( 'Id' => null, diff --git a/web/includes/Object.php b/web/includes/Object.php index 8af4a7676..0ba506e29 100644 --- a/web/includes/Object.php +++ b/web/includes/Object.php @@ -195,8 +195,22 @@ class ZM_Object { return json_encode($json); } + /* Accessor methods that are not in $defaults but may be assigned through set()/changes(). */ + protected static $setters = array(); + + /* Whether a key in data passed to set()/changes() may be dispatched to the method of the + * same name. That data is usually a request array, so its keys are attacker-chosen: only + * field accessors qualify, never methods like save(), delete() or execute(). */ + protected function isSetter($field) { + return array_key_exists($field, $this->defaults) or in_array($field, static::$setters, true); + } + public function set($data) { foreach ($data as $field => $value) { + if (method_exists($this, $field) and !$this->isSetter($field)) { + Warning('Refusing to set '.get_class($this).'::'.$field.', it is not a field'); + continue; + } if (method_exists($this, $field) and is_callable(array($this, $field), false)) { $this->$field($value); } else { @@ -266,6 +280,10 @@ class ZM_Object { } # end if defaults foreach ($new_values as $field => $value) { + if (method_exists($this, $field) and !$this->isSetter($field)) { + Warning('Refusing to change '.get_class($this).'::'.$field.', it is not a field'); + continue; + } if (method_exists($this, $field)) { if (array_key_exists($field, $this->defaults) && is_array($this->defaults[$field]) && isset($this->defaults[$field]['filter_regexp'])) { if (is_array($this->defaults[$field]['filter_regexp'])) { diff --git a/web/includes/Role_Group_Permission.php b/web/includes/Role_Group_Permission.php index 9859dd60e..7857f0e83 100644 --- a/web/includes/Role_Group_Permission.php +++ b/web/includes/Role_Group_Permission.php @@ -6,6 +6,7 @@ require_once('Object.php'); require_once('Group.php'); class Role_Group_Permission extends ZM_Object { + protected static $setters = array('Role', 'Group'); protected static $table = 'Role_Groups_Permissions'; protected $defaults = array( 'Id' => null, diff --git a/web/includes/Role_Monitor_Permission.php b/web/includes/Role_Monitor_Permission.php index cc545fc3d..b2e032d45 100644 --- a/web/includes/Role_Monitor_Permission.php +++ b/web/includes/Role_Monitor_Permission.php @@ -6,6 +6,7 @@ require_once('Object.php'); require_once('Monitor.php'); class Role_Monitor_Permission extends ZM_Object { + protected static $setters = array('Role', 'Monitor'); protected static $table = 'Role_Monitors_Permissions'; protected $defaults = array( 'Id' => null, diff --git a/web/includes/User.php b/web/includes/User.php index dbcd175ea..653ce432f 100644 --- a/web/includes/User.php +++ b/web/includes/User.php @@ -7,6 +7,7 @@ require_once('Monitor_Permission.php'); require_once('User_Preference.php'); class User extends ZM_Object { + protected static $setters = array('Monitor_Permissions', 'Preferences', 'Role'); protected static $table = 'Users'; protected $Id; diff --git a/web/includes/User_Role.php b/web/includes/User_Role.php index d3231368c..ab4f98e4e 100644 --- a/web/includes/User_Role.php +++ b/web/includes/User_Role.php @@ -6,6 +6,7 @@ require_once('Object.php'); require_once('Group.php'); class User_Role extends ZM_Object { + protected static $setters = array('Monitor_Permissions'); protected static $table = 'User_Roles'; protected $defaults = array(