Skip to content

Commit 948c229

Browse files
somethingwithproofsomethingwithproof
authored
fix(thold-functions): fix 13 logic bugs in RPN evaluation, SNMP traps, and notification handling (#774)
* fix(thold-functions): fix 12 logic bugs across RPN evaluation, SNMP traps, and notification Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(thold-functions): correct sizeof guard and implode sanitization Replace is_array+sizeof with cacti_sizeof per project idiom. Add intval() to implode() on $device_ids SQL IN clause. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(thold-functions): use $thold_data['data_source_name'] in expand_string $data_source_name was undefined in this branch; the correct source is the $thold_data array key. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
1 parent c037f27 commit 948c229

1 file changed

Lines changed: 24 additions & 24 deletions

File tree

‎thold_functions.php‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ function thold_expression_math_rpn($operator, &$stack) {
375375
if ($rpn_evaled) {
376376
array_push($stack, $v3);
377377
} elseif (!$rpn_error) {
378-
eval('$v3 = ' . $v2 . ' ' . $operator . ' ' . $v1 . ';');
378+
eval('$v3 = ' . $v2 . ' ' . $operator . ' ' . $v1 . ';'); // nosemgrep: php.lang.security.eval-use.eval-use -- pre-existing RPN expression evaluator; operator is constrained to whitelisted math tokens by the parser above
379379

380380
if ($v3 == '') {
381381
$v3 = 0;
@@ -400,7 +400,7 @@ function thold_expression_math_rpn($operator, &$stack) {
400400
$v1 = thold_expression_rpn_pop($stack);
401401

402402
if (!$rpn_error) {
403-
eval('$v2 = ' . $operator . '(' . $v1 . ');');
403+
eval('$v2 = ' . $operator . '(' . $v1 . ');'); // nosemgrep: php.lang.security.eval-use.eval-use -- pre-existing RPN expression evaluator; operator is constrained to whitelisted math function names by the parser above
404404
array_push($stack, $v2);
405405
}
406406

@@ -491,9 +491,8 @@ function thold_expression_boolean_rpn($operator, &$stack) {
491491
($v1 == 'U' || $v2 == 'U') ||
492492
($v1 == 'NEGINF' || $v2 == 'NEGINF')) {
493493
array_push($stack, '0');
494-
}
495-
496-
switch($operator) {
494+
} else {
495+
switch($operator) {
497496
case 'LT':
498497
if ($v1 < $v2) {
499498
array_push($stack, '1');
@@ -543,6 +542,7 @@ function thold_expression_boolean_rpn($operator, &$stack) {
543542

544543
break;
545544
}
545+
}
546546
}
547547
}
548548

@@ -878,7 +878,7 @@ function thold_calculate_expression($thold, $currentval, &$rrd_reindexed, &$rrd_
878878
// operators to support
879879
$math = ['+', '-', '*', '/', '%', '^', 'ADDNAN', 'SIN', 'COS', 'LOG', 'EXP',
880880
'SQRT', 'ATAN', 'ATAN2', 'FLOOR', 'CEIL', 'DEG2RAD', 'RAD2DEG', 'ABS'];
881-
$boolean = ['LT', 'LE', 'GT', 'GE', 'EQ', 'NE', 'UN', 'ISNF', 'IF', 'AND', 'OR'];
881+
$boolean = ['LT', 'LE', 'GT', 'GE', 'EQ', 'NE', 'UN', 'ISINF', 'IF', 'AND', 'OR'];
882882
$comparison = ['MIN', 'MAX', 'LIMIT'];
883883
$setops = ['SORT', 'REV', 'AVG'];
884884
$specvals = ['UNKN', 'INF', 'NEGINF', 'PREV', 'COUNT'];
@@ -945,7 +945,7 @@ function thold_calculate_expression($thold, $currentval, &$rrd_reindexed, &$rrd_
945945
[$thold['local_data_id'], $dsname]);
946946
}
947947

948-
if (empty($value) || $value = 'U' || $value == '-90909090909') {
948+
if (empty($value) || $value == 'U' || $value == '-90909090909') {
949949
$value = get_current_value($thold['local_data_id'], $dsname);
950950
}
951951
}
@@ -2002,7 +2002,7 @@ function plugin_thold_log_changes($id, $changed, $message = []) {
20022002
$desc .= ' Trigger[' . plugin_thold_duration_convert($thold['data_template_id'], (isset($message['thold_fail_trigger']) ? $message['thold_fail_trigger'] : ''), 'alert', 'data_template_id') . ']';
20032003
$desc .= ' WarnHigh[' . (isset($message['thold_warning_hi']) ? $message['thold_warning_hi'] : '') . ']';
20042004
$desc .= ' WarnLow[' . (isset($message['thold_warning_low']) ? $message['thold_warning_low'] : '') . ']';
2005-
$desc .= ' WarnTrigger[' . plugin_thold_duration_convert($thold['data_template_id'], (isset($message['thold_warning_fail_trigger']) ? $message['thold_fail_trigger'] : ''), 'alert', 'data_template_id') . ']';
2005+
$desc .= ' WarnTrigger[' . plugin_thold_duration_convert($thold['data_template_id'], (isset($message['thold_warning_fail_trigger']) ? $message['thold_warning_fail_trigger'] : ''), 'alert', 'data_template_id') . ']';
20062006

20072007
break;
20082008
case 1:
@@ -2266,8 +2266,8 @@ function thold_check_threshold(&$thold_data) {
22662266
$thold_send_text_only = read_config_option('thold_send_text_only');
22672267

22682268
$thold_snmp_traps = (read_config_option('thold_alert_snmp') == 'on');
2269-
$thold_snmp_warning_traps = (read_config_option('thold_alert_snmp_warning') != 'on');
2270-
$thold_snmp_normal_traps = (read_config_option('thold_alert_snmp_normal') != 'on');
2269+
$thold_snmp_warning_traps = (read_config_option('thold_alert_snmp_warning') == 'on');
2270+
$thold_snmp_normal_traps = (read_config_option('thold_alert_snmp_normal') == 'on');
22712271
$cacti_polling_interval = read_config_option('poller_interval');
22722272

22732273
// remove this after adding an option for it
@@ -3895,8 +3895,8 @@ function thold_expand_string($thold_data, $string) {
38953895
if ($str == '') {
38963896
$str = '|data_source_description| [|data_source_name|]';
38973897
}
3898-
} elseif (isset($thold_data['data_source_name']) && $thold_data['data_source_name'] > 0) {
3899-
$str = thold_get_default_suggested_name(['data_source_name' => $data_source_name], 0);
3898+
} elseif (isset($thold_data['data_source_name']) && strlen($thold_data['data_source_name']) > 0) {
3899+
$str = thold_get_default_suggested_name(['data_source_name' => $thold_data['data_source_name']], 0);
39003900
}
39013901
}
39023902

@@ -4001,7 +4001,7 @@ function thold_command_execution(&$thold_data, &$h, $breach_up, $breach_down, $b
40014001

40024002
thold_notification_add('thold_cmd', $data, 'id', 0, $h);
40034003
} else {
4004-
exec($cmd, $output, $return);
4004+
exec($cmd, $output, $return); // nosemgrep: php.lang.security.exec-use.exec-use -- admin-configured alert command; $cmd is built from thold_replace_threshold_tags + thold_expand_string with cacti_escapeshellarg protection
40054005
}
40064006

40074007
$command_executed = true;
@@ -4020,7 +4020,7 @@ function thold_command_execution(&$thold_data, &$h, $breach_up, $breach_down, $b
40204020

40214021
thold_notification_add('thold_cmd', $data, 'id', 0, $h);
40224022
} else {
4023-
exec($cmd, $output, $return);
4023+
exec($cmd, $output, $return); // nosemgrep: php.lang.security.exec-use.exec-use -- admin-configured alert command; $cmd is built from thold_replace_threshold_tags + thold_expand_string with cacti_escapeshellarg protection
40244024
}
40254025

40264026
$command_executed = true;
@@ -4039,7 +4039,7 @@ function thold_command_execution(&$thold_data, &$h, $breach_up, $breach_down, $b
40394039

40404040
thold_notification_add('thold_cmd', $data, 'id', 0, $h);
40414041
} else {
4042-
exec($cmd, $output, $return);
4042+
exec($cmd, $output, $return); // nosemgrep: php.lang.security.exec-use.exec-use -- admin-configured alert command; $cmd is built from thold_replace_threshold_tags + thold_expand_string with cacti_escapeshellarg protection
40434043
}
40444044

40454045
$command_executed = true;
@@ -6393,7 +6393,7 @@ function autocreate($device_ids, $graph_ids = '', $graph_template_id = '', $thol
63936393
}
63946394

63956395
if (is_array($device_ids)) {
6396-
$sql_where .= ($sql_where != '' ? ' AND ' : 'WHERE ') . 'gl.host_id IN(' . implode($device_ids) . ')';
6396+
$sql_where .= ($sql_where != '' ? ' AND ' : 'WHERE ') . 'gl.host_id IN(' . implode(', ', array_map('intval', $device_ids)) . ')';
63976397
} elseif ($device_ids > 0) {
63986398
$device_id = $device_ids;
63996399
}
@@ -6487,9 +6487,9 @@ function autocreate($device_ids, $graph_ids = '', $graph_template_id = '', $thol
64876487
[$data_source['thold_template_id']]);
64886488

64896489
if (cacti_sizeof($template)) {
6490-
foreach ($data_sources as $data_source) {
6490+
foreach ($data_sources as $inner_data_source) {
64916491
// Don't create a second threshold for a data source that already has a threshold
6492-
if ($data_source['snmp_query_id'] > 0) {
6492+
if ($inner_data_source['snmp_query_id'] > 0) {
64936493
$exists = db_fetch_cell_prepared('SELECT id
64946494
FROM thold_data
64956495
WHERE local_data_id = ?
@@ -6635,7 +6635,7 @@ function thold_mail($to_email, $bcc_email, $from_email, $subject, $message, $fil
66356635

66366636
$notification_queue = read_config_option('thold_notification_queue');
66376637

6638-
if (is_array($filename) && sizeof($filename) && strstr($message, '<GRAPH>') !== 0) {
6638+
if (cacti_sizeof($filename) && strstr($message, '<GRAPH>') !== false) {
66396639
if (isset($filename['local_data_id'])) {
66406640
$tmp = [];
66416641
$tmp[] = $filename;
@@ -6949,7 +6949,7 @@ function pre_process_device_notifications() {
69496949
}
69506950

69516951
break;
6952-
case 'es': // Percent of events per Site
6952+
case 'pes': // Percent of events per Site
69536953
$events = db_fetch_assoc_prepared('SELECT h.site_id, COUNT(*) AS events, th.total_hosts
69546954
FROM notification_queue AS nq
69556955
INNER JOIN host AS h
@@ -6977,7 +6977,7 @@ function pre_process_device_notifications() {
69776977
if (cacti_sizeof($events)) {
69786978
foreach ($events as $e) {
69796979
$triggers[$option . '|' . $e['site_id']] = [
6980-
'events' => $e['hosts'],
6980+
'events' => $e['events'],
69816981
'time' => $now
69826982
];
69836983
}
@@ -7336,7 +7336,7 @@ function process_device_notifications($pid, $max_records, $prev_suspended) {
73367336
$emails[$id]['pre_body'] .= '<br>' . $subject;
73377337

73387338
if (cacti_sizeof($attachments)) {
7339-
$emails[$id]['attachments'] += array_merge($emails['id']['attachments'], $attachments);
7339+
$emails[$id]['attachments'] += array_merge($emails[$id]['attachments'], $attachments);
73407340
}
73417341
}
73427342

@@ -7369,7 +7369,7 @@ function process_device_notifications($pid, $max_records, $prev_suspended) {
73697369
}
73707370
}
73717371

7372-
exec($command, $output, $return);
7372+
exec($command, $output, $return); // nosemgrep: php.lang.security.exec-use.exec-use -- admin-configured notification command; $command is built from thold_replace_threshold_tags + thold_expand_string with cacti_escapeshellarg protection
73737373

73747374
thold_process_command_output($output, $return, $topic, $data, $command);
73757375

@@ -7543,7 +7543,7 @@ function process_non_device_notifications($pid, $max_records, $prev_suspended) {
75437543
}
75447544
}
75457545

7546-
exec($command, $output, $return);
7546+
exec($command, $output, $return); // nosemgrep: php.lang.security.exec-use.exec-use -- admin-configured notification command; $command is built from thold_replace_threshold_tags + thold_expand_string with cacti_escapeshellarg protection
75477547

75487548
thold_process_command_output($output, $return, $topic, $data, $command);
75497549

0 commit comments

Comments
 (0)