Skip to content

Fix string interpolation and platform where 'ip' command (from iproute2) is unavailable. #4734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
f1rmb wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Fix string interpolation and platform where 'ip' command (from iproute2) is unavailable. #4734

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
8a84284
Fix string interpolation and platform where 'ip' command (from iprout…
f1rmb Apr 11, 2026
9681d00
Revert unwanted change.
f1rmb Apr 12, 2026
f34f3ad
Fix indentation stype (space vs tab) for the whole get_network() func…
f1rmb Apr 12, 2026
d627ca2
Remove string interpolation.
f1rmb Apr 14, 2026
88d7c11
Add cmake/config ZM_PATH_ROUTE, and use it in get_network().
f1rmb Apr 14, 2026
21cb8bf
Apply suggestion from @Copilot
connortechnology Apr 14, 2026
7e28665
Use strict operators with strpos().
f1rmb Apr 14, 2026
1527ae6
Merge remote-tracking branch 'origin/fix_string_interpolation_depreca…
f1rmb Apr 14, 2026
09a92b1
Merge branch 'master' into fix_string_interpolation_deprecated_from_P…
f1rmb Apr 18, 2026
9628328
Merge remote-tracking branch 'zoneminder/master' into fix_string_inte…
f1rmb Apr 18, 2026
154ea38
- Revert 'route' usage, use 'netstat' instead: it's works the same on…
f1rmb Apr 18, 2026
a155155
Remove forgotten 'route' checking.
f1rmb Apr 18, 2026
ce8f113
Remove unused variable.
f1rmb Apr 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ mark_as_advanced(
ZM_PATH_UNAME
ZM_PATH_IP
ZM_PATH_IFCONFIG
ZM_PATH_ROUTE
ZM_CONFIG_DIR
ZM_CONFIG_SUBDIR
ZM_DETECT_SYSTEMD
Expand Down Expand Up @@ -198,6 +199,8 @@ set(ZM_PATH_IP "" CACHE PATH
"Full path to compatible ip binary. Leave empty for automatic detection.")
set(ZM_PATH_IFCONFIG "" CACHE PATH
"Full path to compatible ifconfig binary. Leave empty for automatic detection.")
set(ZM_PATH_ROUTE "" CACHE PATH
"Full path to compatible route binary. Leave empty for automatic detection.")
set(ZM_CONFIG_DIR "${CMAKE_INSTALL_FULL_SYSCONFDIR}/zm" CACHE PATH
"Location of ZoneMinder configuration, default system config directory")
set(ZM_CONFIG_SUBDIR "${ZM_CONFIG_DIR}/conf.d" CACHE PATH
Expand Down Expand Up @@ -820,6 +823,17 @@ if(ZM_PATH_IFCONFIG STREQUAL "")
endif()
endif()

# Find the path to an route compatible executable

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says “an route compatible executable”; grammatically this should be “a route compatible executable”.

Suggested change
# Find the path to an route compatible executable
# Find the path to a route compatible executable

Copilot uses AI. Check for mistakes.
if(ZM_PATH_ROUTE STREQUAL "")
find_program(ROUTE_EXECUTABLE route)
if(ROUTE_EXECUTABLE)
set(ZM_PATH_ROUTE "${ROUTE_EXECUTABLE}")
mark_as_advanced(ROUTE_EXECUTABLE)
else()
message(WARNING "Unable to find a compatible route binary.")
endif()
endif()

# Some variables that zm expects
set(ZM_PID "${ZM_RUNDIR}/zm.pid")
set(ZM_CONFIG "${ZM_CONFIG_DIR}/zm.conf")
Expand Down
4 changes: 4 additions & 0 deletions conf.d/01-system-paths.conf.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ ZM_PATH_IP="@ZM_PATH_IP@"
# ZoneMinder will find the ifconfig binary automatically on most systems
ZM_PATH_IFCONFIG="@ZM_PATH_IFCONFIG@"

# Full path to optional route binary or just route to use PATH

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This config comment says ZM_PATH_ROUTE can be set to just route to use PATH, but the current PHP implementation checks file_exists(ZM_PATH_ROUTE) before using it, which will fail for non-absolute values. Either update the comment to require a full path, or update the code to support PATH lookups safely.

Suggested change
# Full path to optional route binary or just route to use PATH
# Full path to optional route binary

Copilot uses AI. Check for mistakes.
# ZoneMinder will find the route binary automatically on most systems
ZM_PATH_ROUTE="@ZM_PATH_ROUTE@"

#Full path to shutdown binary
ZM_PATH_SHUTDOWN="@ZM_PATH_SHUTDOWN@"

Expand Down
1 change: 1 addition & 0 deletions distros/redhat/zoneminder.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ mv -f CxxUrl-%{CxxUrl_version} ./dep/CxxUrl
-DZM_PATH_ARP_SCAN="/usr/sbin/arp-scan" \
-DZM_PATH_IP="/usr/sbin/ip" \
-DZM_PATH_IFCONFIG="/usr/sbin/ifconfig" \
-DZM_PATH_ROUTE="/usr/sbin/route" \
.

%cmake_build
Expand Down
125 changes: 76 additions & 49 deletions web/includes/functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2240,59 +2240,86 @@ function i18n() {
}

function get_networks() {
$interfaces = array();
$interfaces = array();

if (defined('ZM_PATH_IP') and ZM_PATH_IP and file_exists(ZM_PATH_IP)) {
exec(ZM_PATH_IP.' link', $output, $status);
if ( $status ) {
$html_output = implode('<br/>', $output);
ZM\Error("Unable to list network interfaces, status is '$status'. Output was:<br/><br/>$html_output");
} else {
foreach ( $output as $line ) {
if ( preg_match('/^\d+: ([[:alnum:]]+):/', $line, $matches ) ) {
if ( $matches[1] != 'lo' ) {
$interfaces[$matches[1]] = $matches[1];
} else {
ZM\Debug("No match for $line");
}
if (defined('ZM_PATH_IP') and ZM_PATH_IP and file_exists(ZM_PATH_IP)) {
exec(ZM_PATH_IP.' link', $output, $status);
if ( $status ) {
$html_output = implode('<br/>', $output);
ZM\Error("Unable to list network interfaces, status is '$status'. Output was:<br/><br/>$html_output");
} else {
foreach ( $output as $line ) {
if ( preg_match('/^\d+: ([[:alnum:]]+):/', $line, $matches ) ) {
if ( $matches[1] != 'lo' ) {
$interfaces[$matches[1]] = $matches[1];
Comment on lines +2252 to +2254

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ip link interface-name regex only matches [[:alnum:]]+, but Linux interface names can include characters like -, ., and @ (e.g. br-..., eth0.100, veth...@if...). This will silently drop valid interfaces. Consider matching everything up to the trailing : (or using a broader allowed set) and then filtering/normalizing.

Copilot uses AI. Check for mistakes.
} else {
ZM\Debug("Skipping loopback interface $line");
}
}
}
}
}
}
$routes = array();
exec(ZM_PATH_IP.' route', $output, $status);
if ($status) {
$html_output = implode('<br/>', $output);
ZM\Error("Unable to list network interfaces, status is '$status'. Output was:<br/><br/>$html_output");
} else {
foreach ($output as $line) {
if ( preg_match('/^default via [.[:digit:]]+ dev ([[:alnum:]]+)/', $line, $matches) ) {
$interfaces['default'] = $matches[1];
} else if ( preg_match('/^([.[:digit:]]+\/[[:digit:]]+) dev ([[:alnum:]]+)/', $line, $matches) ) {
$interfaces[$matches[2]] .= ' ' . $matches[1];
ZM\Debug("Matched $line: $matches[2] .= $matches[1]");
$routes = array();

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$routes is declared but never used. Please remove it to avoid confusion about intended behavior.

Suggested change
$routes = array();

Copilot uses AI. Check for mistakes.
exec(ZM_PATH_IP.' route', $output, $status);
if ($status) {
$html_output = implode('<br/>', $output);
ZM\Error("Unable to list network interfaces, status is '$status'. Output was:<br/><br/>$html_output");
} else {
ZM\Debug("Didn't match $line");
foreach ($output as $line) {
if ( preg_match('/^default via [.[:digit:]]+ dev ([[:alnum:]]+)/', $line, $matches) ) {
$interfaces['default'] = $matches[1];
} else if ( preg_match('/^([.[:digit:]]+\/[[:digit:]]+) dev ([[:alnum:]]+)/', $line, $matches) ) {

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$interfaces[$matches[2]] .= ... assumes the interface key already exists. If the link parsing missed an interface (or route output references an interface not listed), this will raise a notice and produce a malformed label. Initialize the entry when it’s not set before appending.

Suggested change
} else if ( preg_match('/^([.[:digit:]]+\/[[:digit:]]+) dev ([[:alnum:]]+)/', $line, $matches) ) {
} else if ( preg_match('/^([.[:digit:]]+\/[[:digit:]]+) dev ([[:alnum:]]+)/', $line, $matches) ) {
if ( !isset($interfaces[$matches[2]]) ) {
$interfaces[$matches[2]] = $matches[2];
}

Copilot uses AI. Check for mistakes.
$interfaces[$matches[2]] .= ' ' . $matches[1];
Comment on lines +2268 to +2271

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Route parsing uses the same [[:alnum:]]+ constraint for interface names, which can miss valid interface names containing -, ., or @. This can prevent default-route detection and subnet annotations from working on common setups (bridges/VLANs/veth). Widen the interface-name match here as well.

Copilot uses AI. Check for mistakes.
ZM\Debug("Matched $line: $matches[2] .= $matches[1]");
} else {
ZM\Debug("Didn't match $line");
}
} # end foreach line of output
}
} # end foreach line of output
}
} else if (defined('ZM_PATH_IFCONFIG') and ZM_PATH_IFCONFIG and file_exists(ZM_PATH_IFCONFIG)) {
exec(ZM_PATH_IFCONFIG, $output, $status);
if ($status) {
$html_output = implode("\n", $output);
ZM\Error("Unable to list network interfaces, status is '$status'. Output was:$html_output");
} else {
preg_match("/^([eth|enp][A-z0-9]*)\s+Link\s+encap:([A-z]*)\s+HWaddr\s+([A-z0-9:]*).*".
"inet addr:([0-9.]+).*Bcast:([0-9.]+).*Mask:([0-9.]+).*".
"MTU:([0-9.]+).*Metric:([0-9.]+).*".
"RX packets:([0-9.]+).*errors:([0-9.]+).*dropped:([0-9.]+).*overruns:([0-9.]+).*frame:([0-9.]+).*".
"TX packets:([0-9.]+).*errors:([0-9.]+).*dropped:([0-9.]+).*overruns:([0-9.]+).*carrier:([0-9.]+).*".
"RX bytes:([0-9.]+).*\((.*)\).*TX bytes:([0-9.]+).*\((.*)\)".
"/ims", implode("\n", $output), $regex);

ZM\Debug(print_r( $regex,true));
}
}
return $interfaces;
} else if (defined('ZM_PATH_IFCONFIG') and ZM_PATH_IFCONFIG and file_exists(ZM_PATH_IFCONFIG)) {
$osname = strtolower(php_uname('s'));

exec(ZM_PATH_IFCONFIG, $output, $status);
if ($status) {
$html_output = implode("\n", $output);
ZM\Error("Unable to list network interfaces, status is '$status'. Output was:$html_output");
} else {
exec('ifconfig', $output, $status);

if ($status) {
$html_output = implode("\n", $output);
ZM\Error("Unable to list network interfaces, status is '$status'. Output was:$html_output");
} else {
array_walk($output, function($value, $key) use (&$interfaces) {
$retval = preg_match("/^([A-Za-z0-9]*):\s+flags=([0-9]*)<([A-Z,]*)>.*/ims", $value, $matches);
ZM\Debug(print_r($matches, true));
if (($retval == 1) && (strlen($matches[3]) > 0) &&
(strpos($matches[3], "LOOPBACK") === false) && (strpos($matches[3], "RUNNING") !== false))
{
$interfaces[$matches[1]] = $matches[1];

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ZM\Debug(print_r($matches, true)); runs for every ifconfig line, including non-matching lines, which can generate very noisy logs and extra overhead when debug is enabled. Consider logging only when a line matches (or when a match is rejected), and avoid dumping the full matches array for every line.

Suggested change
ZM\Debug(print_r($matches, true));
if (($retval == 1) && (strlen($matches[3]) > 0) &&
(strpos($matches[3], "LOOPBACK") === false) && (strpos($matches[3], "RUNNING") !== false))
{
$interfaces[$matches[1]] = $matches[1];
if ($retval == 1) {
if ((strlen($matches[3]) > 0) &&
(strpos($matches[3], "LOOPBACK") === false) && (strpos($matches[3], "RUNNING") !== false))
{
ZM\Debug("Accepted interface '".$matches[1]."' with flags '".$matches[3]."'");
$interfaces[$matches[1]] = $matches[1];
} else {
ZM\Debug("Rejected interface '".$matches[1]."' with flags '".$matches[3]."'");
}

Copilot uses AI. Check for mistakes.
}
});

if (defined('ZM_PATH_ROUTE') and ZM_PATH_ROUTE and file_exists(ZM_PATH_ROUTE)) {
// Get default route iface
if (strcmp($osname, 'freebsd') == 0) {
exec(ZM_PATH_ROUTE . " get default | grep interface | awk '{print $2}'", $defaultIface, $status);
} else {
exec(ZM_PATH_ROUTE . " -n | grep '^0.0.0.0' | head -n 1 | awk '{print $8}'", $defaultIface, $status);
}

if ($status) {
$html_output = implode("\n", $output);

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On failure of the route command, the logged output is built from $output (ifconfig output) rather than $defaultIface (route output). This makes the error misleading and may hide the actual failure reason. Use the variable that captured the route output for the error report.

Suggested change
$html_output = implode("\n", $output);
$html_output = implode("\n", $defaultIface);

Copilot uses AI. Check for mistakes.
ZM\Error("Unable to get default ip route, status is '$status'. Output was:$html_output");
} else {
if (isset($defaultIface) && isset($defaultIface[0])) {
$interfaces['default'] = $defaultIface[0];
}
}

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the ifconfig fallback, the code executes ZM_PATH_IFCONFIG and then immediately executes plain ifconfig, overwriting $output. This ignores the explicitly configured ZM_PATH_IFCONFIG (and will fail if ifconfig isn’t in the web server PATH). Use only ZM_PATH_IFCONFIG for the actual interface listing, and remove the redundant second exec/branching.

Suggested change
exec('ifconfig', $output, $status);
if ($status) {
$html_output = implode("\n", $output);
ZM\Error("Unable to list network interfaces, status is '$status'. Output was:$html_output");
} else {
array_walk($output, function($value, $key) use (&$interfaces) {
$retval = preg_match("/^([A-Za-z0-9]*):\s+flags=([0-9]*)<([A-Z,]*)>.*/ims", $value, $matches);
ZM\Debug(print_r($matches, true));
if (($retval == 1) && (strlen($matches[3]) > 0) &&
(strpos($matches[3], "LOOPBACK") === false) && (strpos($matches[3], "RUNNING") !== false))
{
$interfaces[$matches[1]] = $matches[1];
}
});
if (defined('ZM_PATH_ROUTE') and ZM_PATH_ROUTE and file_exists(ZM_PATH_ROUTE)) {
// Get default route iface
if (strcmp($osname, 'freebsd') == 0) {
exec(ZM_PATH_ROUTE . " get default | grep interface | awk '{print $2}'", $defaultIface, $status);
} else {
exec(ZM_PATH_ROUTE . " -n | grep '^0.0.0.0' | head -n 1 | awk '{print $8}'", $defaultIface, $status);
}
if ($status) {
$html_output = implode("\n", $output);
ZM\Error("Unable to get default ip route, status is '$status'. Output was:$html_output");
} else {
if (isset($defaultIface) && isset($defaultIface[0])) {
$interfaces['default'] = $defaultIface[0];
}
}
array_walk($output, function($value, $key) use (&$interfaces) {
$retval = preg_match("/^([A-Za-z0-9]*):\s+flags=([0-9]*)<([A-Z,]*)>.*/ims", $value, $matches);
ZM\Debug(print_r($matches, true));
if (($retval == 1) && (strlen($matches[3]) > 0) &&
(strpos($matches[3], "LOOPBACK") === false) && (strpos($matches[3], "RUNNING") !== false))
{
$interfaces[$matches[1]] = $matches[1];
}
});
if (defined('ZM_PATH_ROUTE') and ZM_PATH_ROUTE and file_exists(ZM_PATH_ROUTE)) {
// Get default route iface
if (strcmp($osname, 'freebsd') == 0) {
exec(ZM_PATH_ROUTE . " get default | grep interface | awk '{print $2}'", $defaultIface, $status);
} else {
exec(ZM_PATH_ROUTE . " -n | grep '^0.0.0.0' | head -n 1 | awk '{print $8}'", $defaultIface, $status);
}
if ($status) {
$html_output = implode("\n", $output);
ZM\Error("Unable to get default ip route, status is '$status'. Output was:$html_output");
} else {
if (isset($defaultIface) && isset($defaultIface[0])) {
$interfaces['default'] = $defaultIface[0];
}

Copilot uses AI. Check for mistakes.
}
}
}
}
return $interfaces;
}

# Returns an array of subnets like 192.168.1.0/24 for a given interface.
Expand Down
7 changes: 4 additions & 3 deletions web/skins/classic/views/onvifprobe.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ function probeProfiles($device_ep, $soapversion, $username, $password) {
<p><label for="interface"><?php echo translate('Interface') ?></label>
<?php
$interfaces = get_networks();
$default_interface = $interfaces['default'];
$default_interface = isset($interfaces['default']) ? $interfaces['default'] : null;
unset($interfaces['default']);

echo htmlSelect('interface', $interfaces,
Expand Down Expand Up @@ -240,8 +240,9 @@ function probeProfiles($device_ep, $soapversion, $username, $password) {
foreach ($detprofiles as $profile) {
$monitor = $camera['monitor'];

$sourceString = "${profile['Name']} : ${profile['Encoding']}" .
" (${profile['Width']}x${profile['Height']} @ ${profile['MaxFPS']}fps ${profile['Transport']})";
$sourceString = $profile['Name'] . ' : ' . $profile['Encoding'] .
' (' . $profile['Width'] . 'x' . $profile['Height'] . ' @ ' . $profile['MaxFPS'] . 'fps ' . $profile['Transport'] . ')';

// copy technical details
$monitor['Width'] = $profile['Width'];
$monitor['Height'] = $profile['Height'];
Expand Down
Loading