From 0b04cc5efce66b60759a50ea9da7d1d0cee1de53 Mon Sep 17 00:00:00 2001 From: Franco Fichtner Date: Tue, 31 Jan 2023 08:49:21 +0100 Subject: [PATCH] system: deal with shell_exec() vs. trim() by wrapping all into shell_safe() --- src/etc/inc/config.inc | 2 +- src/etc/inc/interfaces.inc | 12 ++-- src/etc/inc/system.inc | 16 +++--- src/etc/inc/util.inc | 57 +++++++++---------- src/etc/inc/xmlrpc/legacy.inc | 28 +++++---- .../OPNsense/Core/Api/FirmwareController.php | 2 +- .../mvc/app/library/OPNsense/Backup/Base.php | 2 +- src/opnsense/scripts/dhcp/prefixes.php | 2 +- src/opnsense/scripts/firmware/latest.php | 6 +- src/opnsense/scripts/firmware/product.php | 14 +++-- src/opnsense/scripts/shell/banner.php | 2 +- src/www/crash_reporter.php | 6 +- src/www/guiconfig.inc | 6 +- src/www/widgets/api/plugins/system.inc | 2 +- 14 files changed, 77 insertions(+), 80 deletions(-) diff --git a/src/etc/inc/config.inc b/src/etc/inc/config.inc index e94216932..a6d292dbd 100644 --- a/src/etc/inc/config.inc +++ b/src/etc/inc/config.inc @@ -253,7 +253,7 @@ function make_config_revision_entry($desc = '') if (!empty($_SESSION['Username'])) { $username = $_SESSION['Username']; } else { - $username = '(' . trim(shell_exec('/usr/bin/whoami')) . ')'; + $username = '(' . shell_safe('/usr/bin/whoami') . ')'; } if (!empty($_SERVER['REMOTE_ADDR'])) { diff --git a/src/etc/inc/interfaces.inc b/src/etc/inc/interfaces.inc index f2233de10..5ca71b2c2 100644 --- a/src/etc/inc/interfaces.inc +++ b/src/etc/inc/interfaces.inc @@ -3548,7 +3548,7 @@ function guess_interface_from_ip($ipaddress) return $best_if; } - $ret = exec_command("/sbin/route -n get {$ipaddress} | /usr/bin/awk '/interface/ { print \$2; };'"); + $ret = shell_safe("/sbin/route -n get %s | /usr/bin/awk '/interface/ { print \$2; };'", $ipaddress); if (empty($ret)) { return false; } @@ -3884,10 +3884,8 @@ function get_interfaces_info($include_unlinked = false) $ifinfo['subnetv6'] = $ipv6addr['subnetbits']; } } - } - if (!empty($ifinfo['ifv6'])) { - $aux = trim(shell_exec(exec_safe('/usr/local/sbin/ifctl -6pi %s', $ifinfo['ifv6'])) ?? ''); + $aux = shell_safe('/usr/local/sbin/ifctl -6pi %s', $ifinfo['ifv6']); if (!empty($aux)) { $ifinfo['prefixv6'] = $aux; } @@ -3955,8 +3953,8 @@ function get_interfaces_info($include_unlinked = false) break; } - if (file_exists("/var/run/{$link_type}_{$ifdescr}.pid") && !empty($ifinfo['if'])) { - $sec = intval(trim(shell_exec('/usr/local/opnsense/scripts/interfaces/ppp-uptime.sh ' . escapeshellarg($ifinfo['if'])) ?? '')); + if (file_exists("/var/run/{$link_type}_{$ifdescr}.pid")) { + $sec = intval(shell_safe('/usr/local/opnsense/scripts/interfaces/ppp-uptime.sh %s', $ifinfo['if'])); if ($sec) { $ifinfo['ppp_uptime'] = convert_seconds_to_hms($sec); } @@ -4001,7 +3999,7 @@ function get_interfaces_info($include_unlinked = false) $bridge = link_interface_to_bridge($ifdescr); if ($bridge) { - $bridge_text = shell_exec('/sbin/ifconfig ' . escapeshellarg($bridge)); + $bridge_text = shell_safe('/sbin/ifconfig %s', $bridge); if (stristr($bridge_text, 'blocking') != false) { $ifinfo['bridge'] = "" . gettext("blocking") . " - " . gettext("check for ethernet loops"); $ifinfo['bridgeint'] = $bridge; diff --git a/src/etc/inc/system.inc b/src/etc/inc/system.inc index 011864a85..6afb5149f 100644 --- a/src/etc/inc/system.inc +++ b/src/etc/inc/system.inc @@ -305,7 +305,7 @@ function get_searchdomains() if (isset($syscfg['dnsallowoverride'])) { /* return domains as required by configuration */ - $list = trim(shell_exec('/usr/local/sbin/ifctl -sl')); + $list = shell_safe('/usr/local/sbin/ifctl -sl'); if (!empty($list)) { $search_list = explode("\n", $list); } @@ -339,17 +339,17 @@ function get_nameservers($interface = null, $with_gateway = false) /* only acquire servers provided for this interface */ $realif = get_real_interface($interface); $realifv6 = get_real_interface($interface, 'inet6'); - $list = trim(shell_exec(exec_safe('/usr/local/sbin/ifctl -4nli %s', $realif))); + $list = shell_safe('/usr/local/sbin/ifctl -4nli %s', $realif); if (!empty($list)) { $dns_lists[] = $list; } - $list = trim(shell_exec(exec_safe('/usr/local/sbin/ifctl -6nli %s', $realifv6))); + $list = shell_safe('/usr/local/sbin/ifctl -6nli %s', $realifv6); if (!empty($list)) { $dns_lists[] = $list; } } elseif (isset($syscfg['dnsallowoverride'])) { /* return dynamic servers as required by configuration */ - $list = trim(shell_exec('/usr/local/sbin/ifctl -nl')); + $list = shell_safe('/usr/local/sbin/ifctl -nl'); if (!empty($list)) { $dns_lists = explode("\n", $list); } @@ -385,7 +385,7 @@ function get_nameservers($interface = null, $with_gateway = false) if ($with_gateway) { /* router file is available for connectivity creating nameserver files */ - $gw = trim(shell_exec(exec_safe('/usr/local/sbin/ifctl -%s -ri %s', [$intf[1], $intf[0]]))); + $gw = shell_safe('/usr/local/sbin/ifctl -%s -ri %s', [$intf[1], $intf[0]]); if (is_linklocal($gw) && strpos($gw, '%') === false) { $gw .= "%{$intf[0]}"; } @@ -678,7 +678,7 @@ function system_syslog_start($verbose = false) mwexecf('/usr/local/opnsense/scripts/syslog/generate_certs'); $last_version = @file_get_contents('/var/run/syslog-ng.version'); - $this_version = shell_exec('syslog-ng -V | sha256'); + $this_version = shell_safe('syslog-ng -V | sha256'); if (isvalidpid('/var/run/syslog-ng.pid') && $last_version == $this_version) { mwexecf('/usr/local/sbin/syslog-ng-ctl reload'); @@ -718,7 +718,7 @@ function system_firmware_configure($verbose = false) service_log('Writing firmware setting...', $verbose); /* calculate the effective ABI */ - $args = [ exec_safe('-A %s', trim(shell_exec('opnsense-version -x'))) ]; + $args = [ exec_safe('-A %s', shell_safe('opnsense-version -x')) ]; if (!empty($config['system']['firmware']['mirror'])) { $args[] = exec_safe('-m %s', str_replace('/', '\/', $config['system']['firmware']['mirror'])); @@ -732,7 +732,7 @@ function system_firmware_configure($verbose = false) mwexec('/usr/local/sbin/opnsense-update -sd ' . join(' ', $args)); /* clear the license file when no subscription key is set */ - if (empty(trim(shell_exec('/usr/local/sbin/opnsense-update -K')))) { + if (empty(shell_safe('/usr/local/sbin/opnsense-update -K'))) { @unlink('/usr/local/opnsense/version/core.license'); } diff --git a/src/etc/inc/util.inc b/src/etc/inc/util.inc index 884245233..33283189c 100644 --- a/src/etc/inc/util.inc +++ b/src/etc/inc/util.inc @@ -1,7 +1,7 @@ + * Copyright (C) 2014-2023 Franco Fichtner * Copyright (C) 2010 Ermal Luçi * Copyright (C) 2005-2006 Colin Smith * Copyright (C) 2004-2007 Scott Ullrich @@ -907,11 +907,11 @@ function log_msg($msg, $prio = LOG_NOTICE) syslog($prio, "$page: $msg"); } -function url_safe($format, $args = array()) +function url_safe($format, $args = []) { if (!is_array($args)) { /* just in case there's only one argument */ - $args = array($args); + $args = [$args]; } foreach ($args as $id => $arg) { @@ -932,26 +932,9 @@ function cache_safe($url) return $url; } -/****f* util/exec_command - * NAME - * exec_command - Execute a command and return a string of the result. - * INPUTS - * $command - String of the command to be executed. - * RESULT - * String containing the command's result. - * NOTES - * This function returns the command's stdout and stderr. - ******/ -function exec_command($command) -{ - $output = array(); - exec($command . ' 2>&1', $output); - return(implode("\n", $output)); -} - function mwexec($command, $mute = false) { - $oarr = array(); + $oarr = []; $retval = 0; $garbage = exec("{$command} 2>&1", $oarr, $retval); @@ -973,11 +956,11 @@ function mwexec_bg($command, $mute = false) mwexec("/usr/sbin/daemon -f {$command}", $mute); } -function exec_safe($format, $args = array()) +function exec_safe($format, $args = []) { if (!is_array($args)) { /* just in case there's only one argument */ - $args = array($args); + $args = [$args]; } foreach ($args as $id => $arg) { @@ -987,16 +970,28 @@ function exec_safe($format, $args = array()) return vsprintf($format, $args); } -function mwexecf($format, $args = array(), $mute = false) +function mwexecf($format, $args = [], $mute = false) { return mwexec(exec_safe($format, $args), $mute); } -function mwexecf_bg($format, $args = array(), $mute = false) +function mwexecf_bg($format, $args = [], $mute = false) { mwexec_bg(exec_safe($format, $args), $mute); } +function shell_safe($format, $args = []) +{ + $ret = shell_exec(exec_safe($format, $args)); + + /* shell_exec() has weird semantics */ + if ($ret === false || $ret === null) { + $ret = ''; + } + + /* always trim() result on a string */ + return trim($ret); +} /* check if an alias exists */ function is_alias($name) @@ -1070,7 +1065,7 @@ function ip_in_subnet($addr, $subnet) function is_private_ip($iptocheck) { - foreach (array("10.0.0.0/8", "100.64.0.0/10", "172.16.0.0/12", "192.168.0.0/16") as $private) { + foreach (['10.0.0.0/8', '100.64.0.0/10', '172.16.0.0/12', '192.168.0.0/16'] as $private) { if (ip_in_subnet($iptocheck, $private) == true) { return true; } @@ -1103,20 +1098,20 @@ function format_bytes($bytes) function get_sysctl($names) { if (empty($names)) { - return array(); + return []; } if (is_array($names)) { - $name_list = array(); + $name_list = []; foreach ($names as $name) { $name_list[] = escapeshellarg($name); } } else { - $name_list = array(escapeshellarg($names)); + $name_list = [escapeshellarg($names)]; } exec("/sbin/sysctl -i " . implode(" ", $name_list), $output); - $values = array(); + $values = []; foreach ($output as $line) { $line = explode(": ", $line, 2); if (count($line) == 2) { @@ -1277,7 +1272,7 @@ function is_install_media() function dhcp6c_duid_read() { - $parts = array(); + $parts = []; $skip = 2; if (file_exists('/var/db/dhcp6c_duid')) { diff --git a/src/etc/inc/xmlrpc/legacy.inc b/src/etc/inc/xmlrpc/legacy.inc index a1eaa119d..5ec839d4c 100644 --- a/src/etc/inc/xmlrpc/legacy.inc +++ b/src/etc/inc/xmlrpc/legacy.inc @@ -34,7 +34,11 @@ */ function xmlrpc_publishable_legacy() { - $publish = array('filter_configure_xmlrpc','restore_config_section_xmlrpc','firmware_version_xmlrpc'); + $publish = [ + 'filter_configure_xmlrpc', + 'firmware_version_xmlrpc', + 'restore_config_section_xmlrpc', + ]; return $publish; } @@ -89,7 +93,7 @@ function merge_config_attributes(&$cnf_source, &$cnf_dest) (count($cnf_dest[$cnf_key]) > 0 && isset($cnf_dest[$cnf_key]['@attributes']['uuid'])) // mvc array ) { // (re)set destination array when new or containing a sequenced list of items - $cnf_dest[$cnf_key] = array(); + $cnf_dest[$cnf_key] = []; } merge_config_attributes($cnf_value, $cnf_dest[$cnf_key]); } else { @@ -104,11 +108,11 @@ function merge_config_attributes(&$cnf_source, &$cnf_dest) */ function firmware_version_xmlrpc() { - return array( - 'base' => array('version' => trim(shell_exec('opnsense-version -v base'))), - 'firmware' => array('version' => trim(shell_exec('opnsense-version -v core'))), - 'kernel' => array('version' => trim(shell_exec('opnsense-version -v kernel'))), - ); + return [ + 'base' => ['version' => shell_safe('opnsense-version -v base')], + 'firmware' => ['version' => shell_safe('opnsense-version -v core')], + 'kernel' => ['version' => shell_safe('opnsense-version -v kernel')], + ]; } /** @@ -149,8 +153,8 @@ function restore_config_section_xmlrpc($new_config) // Some sections should just be copied and not merged, namely if there's a risk of attributes being removed // without being seen by the remote (backup) side. // (ipsec, openvpn, nat can probably moved out later by specifying a better path to the attribute) - $sync_full = array('ipsec', 'wol', 'dnsmasq', 'load_balancer', 'openvpn', 'nat', 'dhcpd', 'dhcpv6'); - $sync_full_done = array(); + $sync_full = ['ipsec', 'wol', 'dnsmasq', 'load_balancer', 'openvpn', 'nat', 'dhcpd', 'dhcpv6']; + $sync_full_done = []; foreach ($sync_full as $syncfull) { if (isset($new_config[$syncfull])) { $config[$syncfull] = $new_config[$syncfull]; @@ -166,8 +170,8 @@ function restore_config_section_xmlrpc($new_config) } } - $vipbackup = array(); - $oldvips = array(); + $vipbackup = []; + $oldvips = []; if (isset($new_config['virtualip']['vip']) && isset($config['virtualip']['vip'])) { foreach ($config['virtualip']['vip'] as $vipindex => $vip) { if (!empty($vip['vhid'])) { @@ -209,7 +213,7 @@ function restore_config_section_xmlrpc($new_config) $vipKey = get_unique_vip_key($vip); if (!empty($vip['vhid']) && isset($oldvips[$vipKey])) { $is_changed = false; - foreach (array('password', 'advskew', 'subnet', 'subnet_bits', 'advbase') as $chk_key) { + foreach (['password', 'advskew', 'subnet', 'subnet_bits', 'advbase'] as $chk_key) { if ($oldvips[$vipKey][$chk_key] != $vip[$chk_key]) { $is_changed = true; break; diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Core/Api/FirmwareController.php b/src/opnsense/mvc/app/controllers/OPNsense/Core/Api/FirmwareController.php index 66c1b861f..e492eb849 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Core/Api/FirmwareController.php +++ b/src/opnsense/mvc/app/controllers/OPNsense/Core/Api/FirmwareController.php @@ -841,7 +841,7 @@ class FirmwareController extends ApiControllerBase $backend = new Backend(); $response = array(); - $version = explode(' ', trim(shell_exec('opnsense-version -nv'))); + $version = explode(' ', trim(shell_exec('opnsense-version -nv') ?? '')); foreach (array('product_id' => 0, 'product_version' => 1) as $result => $index) { $response[$result] = !empty($version[$index]) ? $version[$index] : 'unknown'; } diff --git a/src/opnsense/mvc/app/library/OPNsense/Backup/Base.php b/src/opnsense/mvc/app/library/OPNsense/Backup/Base.php index a206ce1aa..f98b6cdf5 100644 --- a/src/opnsense/mvc/app/library/OPNsense/Backup/Base.php +++ b/src/opnsense/mvc/app/library/OPNsense/Backup/Base.php @@ -71,7 +71,7 @@ abstract class Base @unlink($file); if (file_exists("{$file}.enc") && !$retval) { - $version = trim(shell_exec('opnsense-version -Nv')); + $version = trim(shell_exec('opnsense-version -Nv') ?? ''); $result = "---- BEGIN {$tag} ----\n"; $result .= "Version: {$version}\n"; $result .= "Cipher: " . strtoupper($cipher) . "\n"; diff --git a/src/opnsense/scripts/dhcp/prefixes.php b/src/opnsense/scripts/dhcp/prefixes.php index 4a9e5a4db..52f81172f 100755 --- a/src/opnsense/scripts/dhcp/prefixes.php +++ b/src/opnsense/scripts/dhcp/prefixes.php @@ -87,7 +87,7 @@ foreach ($routes as $address => $prefix) { mwexecf('/sbin/route add -inet6 %s %s', [$prefix, $address]); } -$dhcpd_log = trim(shell_exec('opnsense-log -n dhcpd')); +$dhcpd_log = shell_safe('opnsense-log -n dhcpd'); $expires = []; if (empty($dhcpd_log)) { diff --git a/src/opnsense/scripts/firmware/latest.php b/src/opnsense/scripts/firmware/latest.php index 52c30f16a..1c658f7e0 100755 --- a/src/opnsense/scripts/firmware/latest.php +++ b/src/opnsense/scripts/firmware/latest.php @@ -2,7 +2,7 @@ + * Copyright (c) 2021-2023 Franco Fichtner * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -27,9 +27,11 @@ * POSSIBILITY OF SUCH DAMAGE. */ +require_once 'util.inc'; + $changelogfile = '/usr/local/opnsense/changelog/index.json'; -list ($series, $version) = explode(' ', trim(shell_exec('opnsense-version -Vv'))); +list ($series, $version) = explode(' ', shell_safe('opnsense-version -Vv')); $version = explode('_', $version)[0]; $ret = json_decode(@file_get_contents($changelogfile), true); diff --git a/src/opnsense/scripts/firmware/product.php b/src/opnsense/scripts/firmware/product.php index 867c34bdb..086eb6168 100755 --- a/src/opnsense/scripts/firmware/product.php +++ b/src/opnsense/scripts/firmware/product.php @@ -2,7 +2,7 @@ + * Copyright (c) 2021-2023 Franco Fichtner * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -27,18 +27,20 @@ * POSSIBILITY OF SUCH DAMAGE. */ +require_once 'util.inc'; + $metafile = '/usr/local/opnsense/version/core'; $licensefile = $metafile . '.license'; $ret = json_decode(@file_get_contents($metafile), true); if ($ret != null) { - $ret['product_crypto'] = trim(shell_exec('opnsense-version -f')); - $ret['product_latest'] = trim(shell_exec('/usr/local/opnsense/scripts/firmware/latest.php')); - $ret['product_mirror'] = preg_replace('/\/[a-z0-9]{8}(-[a-z0-9]{4}){3}-[a-z0-9]{12}\//i', '/${SUBSCRIPTION}/', trim(shell_exec('opnsense-update -M'))); + $ret['product_crypto'] = shell_safe('opnsense-version -f'); + $ret['product_latest'] = shell_safe('/usr/local/opnsense/scripts/firmware/latest.php'); + $ret['product_mirror'] = preg_replace('/\/[a-z0-9]{8}(-[a-z0-9]{4}){3}-[a-z0-9]{12}\//i', '/${SUBSCRIPTION}/', shell_safe('opnsense-update -M')); $ret['product_time'] = date('D M j H:i:s T Y', filemtime('/usr/local/opnsense/www/index.php')); - $repos = explode("\n", trim(shell_exec('opnsense-verify -l'))); + $repos = explode("\n", shell_safe('opnsense-verify -l')); sort($repos); - $ret['product_log'] = empty(trim(shell_exec('opnsense-update -G'))) ? 0 : 1; + $ret['product_log'] = empty(shell_safe('opnsense-update -G')) ? 0 : 1; $ret['product_repos'] = implode(', ', $repos); $ret['product_check'] = json_decode(@file_get_contents('/tmp/pkg_upgrade.json'), true); $ret['product_license'] = []; diff --git a/src/opnsense/scripts/shell/banner.php b/src/opnsense/scripts/shell/banner.php index 6d670fe5d..ec3b6eb7f 100755 --- a/src/opnsense/scripts/shell/banner.php +++ b/src/opnsense/scripts/shell/banner.php @@ -34,7 +34,7 @@ require_once("interfaces.inc"); require_once("util.inc"); require_once("plugins.inc.d/openssh.inc"); -$version = trim(shell_exec('opnsense-version')); +$version = shell_safe('opnsense-version'); echo "\n*** {$config['system']['hostname']}.{$config['system']['domain']}: {$version} ***\n"; diff --git a/src/www/crash_reporter.php b/src/www/crash_reporter.php index f3da20f35..603e84a7f 100644 --- a/src/www/crash_reporter.php +++ b/src/www/crash_reporter.php @@ -62,7 +62,7 @@ function upload_crash_report($files, $agent) include('head.inc'); -$plugins = implode(' ', explode("\n", shell_exec('pkg info -g "os-*"'))); +$plugins = implode(' ', explode("\n", shell_safe('pkg info -g "os-*"'))); $product = product::getInstance(); $crash_report_header = sprintf( @@ -74,8 +74,8 @@ $crash_report_header = sprintf( $product->hash(), empty($plugins) ? '' : "Plugins $plugins\n", date('r'), - trim(shell_exec('/usr/local/bin/openssl version')), - trim(shell_exec('/usr/local/bin/python3 -V')), + shell_safe('/usr/local/bin/openssl version'), + shell_safe('/usr/local/bin/python3 -V'), PHP_VERSION ); diff --git a/src/www/guiconfig.inc b/src/www/guiconfig.inc index ade951ce2..2d1e059ce 100644 --- a/src/www/guiconfig.inc +++ b/src/www/guiconfig.inc @@ -485,11 +485,7 @@ function get_crash_report() $count = 0; if (file_exists($PHP_errors_log)) { - $total = shell_exec(sprintf( - '/bin/cat %s | /usr/bin/wc -l | /usr/bin/awk \'{ print $1 }\'', - $PHP_errors_log - )); - if ($total > 0) { + if (intval(shell_safe('/bin/cat %s | /usr/bin/wc -l | /usr/bin/awk \'{ print $1 }\'', $PHP_errors_log))) { $count++; } } diff --git a/src/www/widgets/api/plugins/system.inc b/src/www/widgets/api/plugins/system.inc index 7cfc4c0f0..73e756fb6 100644 --- a/src/www/widgets/api/plugins/system.inc +++ b/src/www/widgets/api/plugins/system.inc @@ -165,7 +165,7 @@ function system_api_versions($product) $result[] = sprintf('%s %s-%s', $product['product_name'], $product['product_version'], $product['product_arch']); $result[] = php_uname('s') . ' ' . php_uname('r'); - $result[] = trim(shell_exec('/usr/local/bin/openssl version')); + $result[] = shell_safe('/usr/local/bin/openssl version'); if (!empty($product['product_license']['valid_to'])) { $result[] = gettext('Licensed until') . ' ' . $product['product_license']['valid_to'];