diff --git a/apcupsd_functions.php b/apcupsd_functions.php new file mode 100644 index 0000000..60fb999 --- /dev/null +++ b/apcupsd_functions.php @@ -0,0 +1,77 @@ + 0) { + return $normalized; + } + + return (int)$default; + } +} + +if (!function_exists('apcupsd_get_site_sql_where')) { + function apcupsd_get_site_sql_where($site_id) { + $site_id = apcupsd_normalize_positive_int($site_id, 0); + + if ($site_id > 0) { + return 'site_id = ' . $site_id; + } + + return ''; + } +} + +if (!function_exists('apcupsd_get_autocomplete_rows_limit')) { + function apcupsd_get_autocomplete_rows_limit($rows) { + return apcupsd_normalize_positive_int($rows, 1); + } +} + +if (!function_exists('apcupsd_escape_shellcmd')) { + function apcupsd_escape_shellcmd($value) { + if (function_exists('cacti_escapeshellcmd')) { + return cacti_escapeshellcmd($value); + } + + return escapeshellcmd($value); + } +} + +if (!function_exists('apcupsd_escape_shellarg')) { + function apcupsd_escape_shellarg($value) { + if (function_exists('cacti_escapeshellarg')) { + return cacti_escapeshellarg($value); + } + + return escapeshellarg($value); + } +} + +if (!function_exists('apcupsd_build_apcaccess_command')) { + function apcupsd_build_apcaccess_command($binary_path, $hostname, $port) { + $hostname = trim((string)$hostname); + $port = apcupsd_normalize_positive_int($port, 0); + + if ($binary_path === '' || $hostname === '' || $port < 1 || $port > 65535) { + return false; + } + + return apcupsd_escape_shellcmd($binary_path) . ' -u -h ' . apcupsd_escape_shellarg($hostname . ':' . $port); + } +} diff --git a/poller_apcupsd.php b/poller_apcupsd.php index 79b607d..8269c6a 100644 --- a/poller_apcupsd.php +++ b/poller_apcupsd.php @@ -39,6 +39,7 @@ require_once($config['base_path'] . '/lib/template.php'); require_once($config['base_path'] . '/lib/utility.php'); include('./plugins/apcupsd/database.php'); +require_once('./plugins/apcupsd/apcupsd_functions.php'); /* process calling arguments */ $parms = $_SERVER['argv']; @@ -363,13 +364,22 @@ function collect_ups_data($ups) { } } - $command = $found_path . 'apcaccess -u -h ' . $ups['hostname'] . ':' . $ups['port']; + $command = apcupsd_build_apcaccess_command($found_path . 'apcaccess', $ups['hostname'], $ups['port']); $output = array(); $return = 0; $ups_status = 1; + if ($command === false) { + db_execute_prepared('UPDATE apcupsd_ups + SET status = 1, error_message = ? + WHERE id = ?', + array(__('Invalid apcupsd hostname or port configuration', 'apcupsd'), $ups['id'])); + + return 1; + } + $results = exec($command, $output, $return); if ($return > 0) { @@ -464,4 +474,3 @@ function display_help() { print "usage: \n"; print "poller_apcups.php [--force] [--debug]\n"; } - diff --git a/tests/e2e/test_security_wiring.php b/tests/e2e/test_security_wiring.php new file mode 100644 index 0000000..a4b1fe5 --- /dev/null +++ b/tests/e2e/test_security_wiring.php @@ -0,0 +1,59 @@ + 0 ? 1 : 0); diff --git a/tests/integration/test_poller_command_security.php b/tests/integration/test_poller_command_security.php new file mode 100644 index 0000000..e7ac914 --- /dev/null +++ b/tests/integration/test_poller_command_security.php @@ -0,0 +1,51 @@ + 0 ? 1 : 0); diff --git a/tests/test_layout_wrapper.php b/tests/test_layout_wrapper.php new file mode 100644 index 0000000..10056ea --- /dev/null +++ b/tests/test_layout_wrapper.php @@ -0,0 +1,65 @@ +content->bottom', $events === array('top_header', 'content', 'bottom_footer')); + +$upses_source = file_get_contents(__DIR__ . '/../upses.php'); + +assert_true( + "edit action uses shared layout helper", + strpos($upses_source, "apcupsd_render_with_layout('ups_edit');") !== false +); +assert_true( + "default action uses shared layout helper", + strpos($upses_source, "apcupsd_render_with_layout('upses');") !== false +); + +echo "\n"; +echo "Results: $pass passed, $fail failed\n"; + +exit($fail > 0 ? 1 : 0); diff --git a/tests/unit/test_security_helpers.php b/tests/unit/test_security_helpers.php new file mode 100644 index 0000000..0716966 --- /dev/null +++ b/tests/unit/test_security_helpers.php @@ -0,0 +1,70 @@ + 0 ? 1 : 0); diff --git a/ui_helpers.php b/ui_helpers.php new file mode 100644 index 0000000..0e789bd --- /dev/null +++ b/ui_helpers.php @@ -0,0 +1,19 @@ + __('Delete', 'apcupsd'), @@ -174,21 +176,13 @@ break; case 'ajax_hosts': - $sql_where = ''; - if (get_request_var('site_id') > 0) { - $sql_where = 'site_id = ' . get_request_var('site_id'); - } - - get_allowed_ajax_hosts(false, false, $sql_where); + get_filter_request_var('site_id', FILTER_VALIDATE_INT); + get_allowed_ajax_hosts(false, false, apcupsd_get_site_sql_where(get_request_var('site_id'))); break; case 'ajax_hosts_noany': - $sql_where = ''; - if (get_request_var('site_id') > 0) { - $sql_where = 'site_id = ' . get_request_var('site_id'); - } - - get_allowed_ajax_hosts(false, true, $sql_where); + get_filter_request_var('site_id', FILTER_VALIDATE_INT); + get_allowed_ajax_hosts(false, true, apcupsd_get_site_sql_where(get_request_var('site_id'))); break; case 'ajax_locations': @@ -200,23 +194,15 @@ FROM mysql.time_zone_name WHERE Name LIKE ? ORDER BY Name - LIMIT ' . read_config_option('autocomplete_rows'), + LIMIT ' . apcupsd_get_autocomplete_rows_limit(read_config_option('autocomplete_rows')), array('%' . get_nfilter_request_var('term') . '%'))); break; case 'edit': - top_header(); - - ups_edit(); - - bottom_footer(); + apcupsd_render_with_layout('ups_edit'); break; default: - top_header(); - - upses(); - - bottom_footer(); + apcupsd_render_with_layout('upses'); break; } @@ -235,7 +221,7 @@ function form_save() { $save['type_id'] = form_input_validate(get_nfilter_request_var('type_id'), 'type_id', '', true, 3); $save['name'] = form_input_validate(get_nfilter_request_var('name'), 'name', '', false, 3); $save['description'] = form_input_validate(get_nfilter_request_var('description'), 'description', '', true, 3); - $save['site_id'] = form_input_validate(get_nfilter_request_var('site_id'), 'site_id', '', true, 3); + $save['site_id'] = form_input_validate(get_nfilter_request_var('site_id'), 'site_id', '^[0-9]+$', true, 3); if ($save['type_id'] == 1) { $save['hostname'] = form_input_validate(get_nfilter_request_var('hostname'), 'hostname', '', true, 3); @@ -243,7 +229,7 @@ function form_save() { $save['hostname'] = form_input_validate(get_nfilter_request_var('snmp_hostname'), 'snmp_hostname', '', true, 3); } - $save['port'] = form_input_validate(get_nfilter_request_var('port'), 'port', '', true, 3); + $save['port'] = form_input_validate(get_nfilter_request_var('port'), 'port', '^[0-9]+$', true, 3); $save['enabled'] = isset_request_var('enabled') ? 'on':''; $save['snmp_version'] = form_input_validate(get_nfilter_request_var('snmp_version'), 'snmp_version', '', true, 3); @@ -257,7 +243,7 @@ function form_save() { $save['snmp_context'] = form_input_validate(get_nfilter_request_var('snmp_context'), 'snmp_context', '', true, 3); $save['snmp_engine_id'] = form_input_validate(get_nfilter_request_var('snmp_engine_id'), 'snmp_engine_id', '', true, 3); - $save['snmp_port'] = form_input_validate(get_nfilter_request_var('snmp_port'), 'snmp_port', '', true, 3); + $save['snmp_port'] = form_input_validate(get_nfilter_request_var('snmp_port'), 'snmp_port', '^[0-9]+$', true, 3); $save['snmp_timeout'] = form_input_validate(get_nfilter_request_var('snmp_timeout'), 'snmp_timeout', '', true, 3); if ($save['host_id'] > 0) {