fix: sanitize monitor Device path to prevent command injection (GHSA-g66m-77fq-79v9)

The Device field from the Monitors table was interpolated directly into
shell commands (qx(), backticks, exec()) without sanitization, allowing
authenticated users with monitor-edit permissions to execute arbitrary
commands as www-data via the Device Path field.

Defense in depth:
- Input validation: reject Device values not matching /^\/dev\/[\w\/.\-]+$/
  at save time in both web UI and REST API
- Output sanitization: use escapeshellarg() in PHP and quote validated
  values in Perl at every shell execution point

Affected locations:
- scripts/ZoneMinder/lib/ZoneMinder/Monitor.pm (control, zmcControl)
- scripts/zmpkg.pl.in (system startup)
- web/includes/Monitor.php (zmcControl)
- web/includes/functions.php (zmcStatus, zmcCheck, validDevicePath)
- web/includes/actions/monitor.php (save action)
- web/api/app/Model/Monitor.php (daemonControl, validation rules)
- web/api/app/Controller/MonitorsController.php (daemonStatus)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
pull/4693/head
Isaac Connor 2026-03-08 13:19:03 -04:00
parent 11039c8fd5
commit 419846c875
7 changed files with 61 additions and 16 deletions

View File

@ -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};
}

View File

@ -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}");
}

View File

@ -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

View File

@ -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);

View File

@ -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') {

View File

@ -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).</br>';
return;
}
}
if (!$newMonitor['ManufacturerId'] and ($newMonitor['Manufacturer'] != '')) {
# Need to add a new Manufacturer entry
$newManufacturer = ZM\Manufacturer::find_one(array('Name'=>$newMonitor['Manufacturer']));

View File

@ -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 '';