fix: require System edit to change a monitor's event commands refs GHSA-vcrj-35c6-g97c

zmc runs a monitor's EventStartCommand and EventEndCommand at the start
and end of each event. Both the classic save and bulk edit actions and
the API monitor add and edit authorized only on the monitor, so a user
with edit on one monitor and no System permission could set a command
for zmc to run as the ZoneMinder service user.

Changing either field now needs System=Edit, as Filter::canEdit()
already requires for AutoExecuteCmd. Monitor::commandFieldChangesNotAllowed()
reports the command fields a change would alter; submitting the current
value is not a change, so other users can still save the rest of the
monitor. The monitor form shows the fields read-only to them. Also
close the EventEndCommand list item.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

web/api/app/Controller/MonitorsController.php

@@ -126,6 +126,17 @@ class MonitorsController extends AppController {
  *
  * @return void
  */
+  // The EventStartCommand / EventEndCommand values the request supplies, if any.
+  private function requestCommandFields() {
+    require_once __DIR__ .'/../../../includes/Monitor.php';
+    $fields = array();
+    foreach (ZM\Monitor::$commandFields as $field) {
+      $value = $this->requestField('Monitor', $field);
+      if ($value !== null) $fields[$field] = $value;
+    }
+    return $fields;
+  }
+
   public function add() {
     if ( $this->request->is('post') ) {
 
@@ -135,6 +146,12 @@ class MonitorsController extends AppController {
         throw new UnauthorizedException(__('Insufficient privileges'));
         return;
       }
+      // A new monitor's command fields start empty, so any value is a change.
+      require_once __DIR__ .'/../../../includes/Monitor.php';
+      $new_monitor = new ZM\Monitor();
+      if (count($new_monitor->commandFieldChangesNotAllowed($this->requestCommandFields()))) {
+        throw new UnauthorizedException(__('Setting the event commands requires System edit permission'));
+      }
       $this->pinRequestId($this->Monitor, null);
       $this->Monitor->create();
       if ($this->Monitor->save($this->request->data) ) {
@@ -178,6 +195,9 @@ class MonitorsController extends AppController {
       throw new UnauthorizedException(__('Insufficient Privileges'));
       return;
     }
+    if (count($zm_monitor->commandFieldChangesNotAllowed($this->requestCommandFields()))) {
+      throw new UnauthorizedException(__('Changing the event commands requires System edit permission'));
+    }
 
     $this->pinRequestId($this->Monitor, $id);
     $message = '';

web/includes/Monitor.php

@@ -953,6 +953,25 @@ class Monitor extends ZM_Object {
     return $this->connKey;
   }
 
+  // Fields whose value zmc runs as a command. Setting one is equivalent to shell access
+  // on the server, so it needs System=Edit, not just edit on this monitor, the same rule
+  // Filter::canEdit() applies to AutoExecuteCmd. refs GHSA-vcrj-35c6-g97c
+  public static $commandFields = array('EventStartCommand', 'EventEndCommand');
+
+  // The command fields $data would change on this monitor that the user may not set.
+  // Submitting the current value is not a change, since the monitor form always posts it.
+  public function commandFieldChangesNotAllowed($data, $u=null) {
+    global $user;
+    if (!$u) $u = $user;
+    if (!$u or $u->System() == 'Edit') return array();
+    $refused = array();
+    foreach (self::$commandFields as $field) {
+      if (!array_key_exists($field, $data)) continue;
+      if ((string)$data[$field] !== (string)$this->{$field}()) $refused[] = $field;
+    }
+    return $refused;
+  }
+
   function canEdit($u=null) {
     global $user;
     if ($u===null or $u->Id() == $user->Id())

web/includes/actions/monitor.php

@@ -153,6 +153,13 @@ if ($action == 'save') {
   if (!empty($newMonitor['ModelId']) and empty($newMonitor['Model']))
     unset($newMonitor['Model']);
 
+  $refused = $monitor->commandFieldChangesNotAllowed($newMonitor);
+  if (count($refused)) {
+    $error_message .= 'Changing '.implode(', ', $refused).' requires System edit permission.<br/>';
+    ZM\Warning('Refused to save monitor '.$mid.': '.implode(', ', $refused).' requires System edit permission');
+    return;
+  }
+
   $changes = $monitor->changes($newMonitor);
   ZM\Debug('Changes: '. print_r($changes, true));
   $restart = false;

web/includes/actions/monitors.php

@@ -33,6 +33,12 @@ if ($action == 'save') {
       continue;
     }
     $Monitor = new ZM\Monitor($mid);
+    // Before stopping it, so a refused save leaves the monitor running.
+    $refused = $Monitor->commandFieldChangesNotAllowed($_REQUEST['newMonitor']);
+    if (count($refused)) {
+      $error_message .= 'Changing '.implode(', ', $refused).' requires System edit permission.<br/>';
+      continue;
+    }
     if ($Monitor->Type() != 'WebSite') {
       $Monitor->zmcControl('stop');
     }

web/skins/classic/views/monitor.php

@@ -1328,14 +1328,18 @@ echo htmlSelect('newMonitor[OutputContainer]', $videowriter_containers, $monitor
               <input type="hidden" name="newMonitor[RecordAudio]" value="<?php echo $monitor->RecordAudio() ? 1 : 0 ?>"/>
 <?php } ?>
             </li>
+<?php
+      // zmc runs these as commands, so only System editors may change them.
+      $commandReadonly = canEdit('System') ? '' : ' readonly="readonly" title="'.validHtmlStr(translate('Requires System edit permission')).'"';
+?>
             <li class="EventStartCommand">
               <label><?php echo translate('Event Start Command') ?></label>
-              <input type="text" name="newMonitor[EventStartCommand]" value="<?php echo validHtmlStr($monitor->EventStartCommand()) ?>" />
+              <input type="text" name="newMonitor[EventStartCommand]" value="<?php echo validHtmlStr($monitor->EventStartCommand()) ?>"<?php echo $commandReadonly ?>/>
             </li>
             <li class="EventEndCommand">
               <label><?php echo translate('Event End Command') ?></label>
-              <input type="text" name="newMonitor[EventEndCommand]" value="<?php echo validHtmlStr($monitor->EventEndCommand()) ?>" />
-            <li>
+              <input type="text" name="newMonitor[EventEndCommand]" value="<?php echo validHtmlStr($monitor->EventEndCommand()) ?>"<?php echo $commandReadonly ?>/>
+            </li>
 <?php
       break;
     }