diff --git a/scripts/ZoneMinder/lib/ZoneMinder/Monitor.pm b/scripts/ZoneMinder/lib/ZoneMinder/Monitor.pm index 3f1d668f0..f9c21daec 100644 --- a/scripts/ZoneMinder/lib/ZoneMinder/Monitor.pm +++ b/scripts/ZoneMinder/lib/ZoneMinder/Monitor.pm @@ -347,12 +347,20 @@ sub control { my $command = shift; my $process = shift; + my $valid_device = (defined $monitor->{Device} and $monitor->{Device} =~ /^\/dev\/[\w\/.\-]+$/); + if ($monitor->{Type} eq 'Local' and !$valid_device) { + Error("Invalid device path rejected: $monitor->{Device}"); + return; + } elsif (!$valid_device and defined $monitor->{Device} and length($monitor->{Device})) { + Warning("Monitor $$monitor{Id} has invalid device path: $monitor->{Device}"); + } + if ($command eq 'stop') { if ($process) { ZoneMinder::General::runCommand("zmdc.pl stop $process -m $$monitor{Id}"); } else { if ($monitor->{Type} eq 'Local') { - ZoneMinder::General::runCommand('zmdc.pl stop zmc -d '.$monitor->{Device}); + ZoneMinder::General::runCommand("zmdc.pl stop zmc -d '$monitor->{Device}'"); } else { ZoneMinder::General::runCommand('zmdc.pl stop zmc -m '.$monitor->{Id}); } @@ -362,7 +370,7 @@ sub control { ZoneMinder::General::runCommand("zmdc.pl start $process -m $$monitor{Id}"); } else { if ($monitor->{Type} eq 'Local') { - ZoneMinder::General::runCommand('zmdc.pl start zmc -d '.$monitor->{Device}); + ZoneMinder::General::runCommand("zmdc.pl start zmc -d '$monitor->{Device}'"); } else { ZoneMinder::General::runCommand('zmdc.pl start zmc -m '.$monitor->{Id}); } @@ -372,7 +380,7 @@ sub control { ZoneMinder::General::runCommand("zmdc.pl restart $process -m $$monitor{Id}"); } else { if ($monitor->{Type} eq 'Local') { - ZoneMinder::General::runCommand('zmdc.pl restart zmc -d '.$monitor->{Device}); + ZoneMinder::General::runCommand("zmdc.pl restart zmc -d '$monitor->{Device}'"); } else { ZoneMinder::General::runCommand('zmdc.pl restart zmc -m '.$monitor->{Id}); } @@ -498,7 +506,11 @@ sub zmcControl { if ((!$ZoneMinder::Config{ZM_SERVER_ID}) or ( $$self{ServerId} and ($ZoneMinder::Config{ZM_SERVER_ID}==$$self{ServerId}) )) { if ($$self{Type} eq 'Local') { - $zmcArgs .= '-d '.$self->{Device}; + if (!defined $$self{Device} or $$self{Device} !~ /^\/dev\/[\w\/.\-]+$/) { + Error("Invalid device path rejected: $$self{Device}"); + return; + } + $zmcArgs .= "-d '$$self{Device}'"; } else { $zmcArgs .= '-m '.$self->{Id}; } diff --git a/scripts/zmpkg.pl.in b/scripts/zmpkg.pl.in index 0bb22a9a9..0f97e6d80 100644 --- a/scripts/zmpkg.pl.in +++ b/scripts/zmpkg.pl.in @@ -211,7 +211,11 @@ if ( $command =~ /^(?:start|restart)$/ ) { foreach my $monitor (@monitors) { if (($monitor->{Capturing} ne 'None') and ($monitor->{Type} ne 'WebSite')) { if ( $monitor->{Type} eq 'Local' ) { - runCommand("zmdc.pl start zmc -d $monitor->{Device}"); + if ($monitor->{Device} !~ /^\/dev\/[\w\/.\-]+$/) { + Error("Invalid device path rejected for monitor $monitor->{Id}: $monitor->{Device}"); + next; + } + runCommand("zmdc.pl start zmc -d '$monitor->{Device}'"); } else { runCommand("zmdc.pl start zmc -m $monitor->{Id}"); } diff --git a/web/api/app/Controller/MonitorsController.php b/web/api/app/Controller/MonitorsController.php index 1feb901a0..90935fb0f 100644 --- a/web/api/app/Controller/MonitorsController.php +++ b/web/api/app/Controller/MonitorsController.php @@ -342,9 +342,9 @@ class MonitorsController extends AppController { // Pass -d for local, otherwise -m if ( $monitor[0]['Type'] == 'Local' ) { - $args = '-d '. $monitor[0]['Device']; + $args = '-d '. escapeshellarg($monitor[0]['Device']); } else { - $args = '-m '. $monitor[0]['Id']; + $args = '-m '. escapeshellarg($monitor[0]['Id']); } // Build the command, and execute it diff --git a/web/api/app/Model/Monitor.php b/web/api/app/Model/Monitor.php index 98a874904..8fdfb88a7 100644 --- a/web/api/app/Model/Monitor.php +++ b/web/api/app/Model/Monitor.php @@ -60,6 +60,14 @@ class Monitor extends AppModel { 'OutputCodec' => array ( 'rule' => array('inList', array (0,27,173,167,226)), 'message'=>'Invalid value. Should be one of these integer values: 0(auto), 27(h264), 173(h265/hvec), 167(vp9), 226(av1)' + ), + 'Device' => array( + 'validPath' => array( + 'rule' => array('custom', '#^(/dev/[\w/.\-]+)?$#'), + 'message' => 'Invalid device path. Must be a valid /dev/ path (e.g. /dev/video0).', + 'allowEmpty' => true, + 'required' => false, + ), ) ); @@ -183,11 +191,11 @@ class Monitor extends AppModel { foreach ($daemons as $daemon) { $args = ''; if ($daemon == 'zmc' and $monitor['Type'] == 'Local') { - $args = '-d ' . $monitor['Device']; + $args = '-d ' . escapeshellarg($monitor['Device']); } else if ($daemon == 'zmcontrol.pl') { - $args = '--id '.$monitor['Id']; + $args = '--id '.escapeshellarg($monitor['Id']); } else { - $args = '-m ' . $monitor['Id']; + $args = '-m ' . escapeshellarg($monitor['Id']); } $shellcmd = escapeshellcmd(ZM_PATH_BIN.'/zmdc.pl '.$command.' '.$daemon.' '.$args); diff --git a/web/includes/Monitor.php b/web/includes/Monitor.php index 47bd8be79..17eb2cb94 100644 --- a/web/includes/Monitor.php +++ b/web/includes/Monitor.php @@ -646,9 +646,9 @@ class Monitor extends ZM_Object { } if ((!defined('ZM_SERVER_ID')) or ( property_exists($this, 'ServerId') and (ZM_SERVER_ID==$this->{'ServerId'}) )) { if ($this->Type() == 'Local') { - $zmcArgs = '-d '.$this->{'Device'}; + $zmcArgs = '-d '.escapeshellarg($this->{'Device'}); } else { - $zmcArgs = '-m '.$this->{'Id'}; + $zmcArgs = '-m '.escapeshellarg($this->{'Id'}); } if ($mode == 'stop') { diff --git a/web/includes/actions/monitor.php b/web/includes/actions/monitor.php index 706637c82..0ef133d41 100644 --- a/web/includes/actions/monitor.php +++ b/web/includes/actions/monitor.php @@ -58,6 +58,15 @@ if ($action == 'save') { # For convenience $newMonitor = $_REQUEST['newMonitor']; + # Validate Device path to prevent command injection (CVE-worthy) + if (!empty($newMonitor['Device'])) { + $newMonitor['Device'] = validDevicePath($newMonitor['Device']); + if ($newMonitor['Device'] === '') { + $error_message .= 'Invalid device path. Must be a valid /dev/ path (e.g. /dev/video0).
'; + return; + } + } + if (!$newMonitor['ManufacturerId'] and ($newMonitor['Manufacturer'] != '')) { # Need to add a new Manufacturer entry $newManufacturer = ZM\Manufacturer::find_one(array('Name'=>$newMonitor['Manufacturer'])); diff --git a/web/includes/functions.php b/web/includes/functions.php index 5b4eb9cf7..b5dbc08a9 100644 --- a/web/includes/functions.php +++ b/web/includes/functions.php @@ -755,9 +755,9 @@ function daemonStatus($daemon, $args=false) { function zmcStatus($monitor) { if ( $monitor['Type'] == 'Local' ) { - $zmcArgs = '-d '.$monitor['Device']; + $zmcArgs = '-d '.escapeshellarg($monitor['Device']); } else { - $zmcArgs = '-m '.$monitor['Id']; + $zmcArgs = '-m '.escapeshellarg($monitor['Id']); } return daemonStatus('zmc', $zmcArgs); } @@ -776,9 +776,9 @@ function daemonCheck($daemon=false, $args=false) { function zmcCheck($monitor) { if ( $monitor['Type'] == 'Local' ) { - $zmcArgs = '-d '.$monitor['Device']; + $zmcArgs = '-d '.escapeshellarg($monitor['Device']); } else { - $zmcArgs = '-m '.$monitor['Id']; + $zmcArgs = '-m '.escapeshellarg($monitor['Id']); } return daemonCheck('zmc', $zmcArgs); } @@ -1880,6 +1880,18 @@ function validNum( $input ) { return preg_replace('/[^\d.-]/', '', $input); } +// For device path strings - must be a valid Unix device path +function validDevicePath($input) { + if (is_null($input) || $input === '') return ''; + // Only allow typical device paths: /dev/video0, /dev/v4l/by-id/..., etc. + // Reject any shell metacharacters + if (!preg_match('#^/dev/[\w/.\-]+$#', $input)) { + ZM\Warning("Invalid device path rejected: ".validHtmlStr($input)); + return ''; + } + return $input; +} + // For general strings function validStr($input) { if (is_null($input)) return '';