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 4 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
121 changes: 72 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,82 @@ 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("No match for $line");
Comment thread
connortechnology marked this conversation as resolved.
Outdated
}
}
}
}
}
}
$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))

Copilot AI Apr 14, 2026

Copy link

Choose a reason for hiding this comment

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

The strpos checks against the flags string use loose comparisons (== false / != false), which mis-handle matches at position 0 (e.g., strpos(...) returning 0). Use strict comparisons (=== false and !== false) so LOOPBACK/RUNNING detection is correct in all cases.

Suggested change
(strpos($matches[3], "LOOPBACK") == false) && (strpos($matches[3], "RUNNING") != false))
(strpos($matches[3], "LOOPBACK") === false) && (strpos($matches[3], "RUNNING") !== false))

Copilot uses AI. Check for mistakes.
{
$interfaces[$matches[1]] = $matches[1];
}
});

if (strcmp($osname, 'freebsd') == 0) {
// Get default route iface
exec("route get default | grep interface | awk '{print $2}'", $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 AI Apr 14, 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 FreeBSD default-route block, the error log builds $html_output from $output (the ifconfig output), not from $defaultIface (the route get default output), so failures will report the wrong command output. Also, this uses a shell pipeline (route | grep | awk) and unqualified command names, which is brittle if the web server PATH doesn’t include these tools; prefer executing the route command directly and parsing in PHP (or using configured full paths).

Suggested change
exec("route get default | grep interface | awk '{print $2}'", $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];
$defaultIface = array();
$routeCommand = 'route';
if (is_executable('/sbin/route')) {
$routeCommand = '/sbin/route';
} else if (is_executable('/usr/sbin/route')) {
$routeCommand = '/usr/sbin/route';
}
exec($routeCommand.' get default', $defaultIface, $status);
if ($status) {
$html_output = implode("\n", $defaultIface);
ZM\Error("Unable to get default ip route, status is '$status'. Output was:$html_output");
} else {
foreach ($defaultIface as $routeLine) {
if (preg_match('/^\s*interface:\s*(\S+)/', $routeLine, $matches)) {
$interfaces['default'] = $matches[1];
break;
}

Copilot uses AI. Check for mistakes.
}

Copilot AI Apr 14, 2026

Copy link

Choose a reason for hiding this comment

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

The ifconfig fallback runs exec(ZM_PATH_IFCONFIG, ...) and then immediately runs exec('ifconfig', ...), overwriting $output. If ZM_PATH_IFCONFIG succeeds but ifconfig isn’t in the web server PATH (common for /sbin tools), this will incorrectly fail. Use ZM_PATH_IFCONFIG for the actual call (and remove the redundant first/second exec).

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 (strcmp($osname, 'freebsd') == 0) {
// Get default route iface
exec("route get default | grep interface | awk '{print $2}'", $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 (strcmp($osname, 'freebsd') == 0) {
// Get default route iface
exec("route get default | grep interface | awk '{print $2}'", $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.

@connortechnology connortechnology Apr 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There are actually security issues around just running things and expecting PATH to find them. Sometimes the ZM admin is NOT the system admin. So system admin sets ZM_PATH_IFCONFIG and ZM admin just has to live with it.

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