Skip to content

fix(security): defense-in-depth hardening for plugin_thold #767

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

Draft
somethingwithproof wants to merge 7 commits into
base: develop
Choose a base branch
from
Draft

fix(security): defense-in-depth hardening for plugin_thold #767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2d6b8f2
fix(security): defense-in-depth hardening for plugin_thold
somethingwithproof Apr 9, 2026
a276abf
fix(js): migrate deprecated jQuery shorthand events to .on()/.off()
somethingwithproof Apr 9, 2026
cabb181
fix(ci): Dependabot composer ecosystem, CodeQL PHP coverage
somethingwithproof Apr 10, 2026
78b6ba9
fix: execute addThresholdTemplate POST directly instead of binding click
somethingwithproof Apr 10, 2026
652419f
fix: pass unserialize options to cacti_unserialize
somethingwithproof Apr 11, 2026
7191dbc
chore: drop local .omc ignore noise
somethingwithproof Apr 11, 2026
e591b10
fix(security): harden notify list form and filter handling
somethingwithproof Apr 11, 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
44 changes: 22 additions & 22 deletions notify_lists.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ function form_actions() {
<input type='hidden' name='action' value='actions'>
<input type='hidden' name='save_list' value='1'>
<input type='hidden' name='selected_items' value='" . (isset($array) ? serialize($array) : '') . "'>
<input type='hidden' name='drp_action' value='" . get_request_var('drp_action') . "'>
<input type='hidden' name='drp_action' value='" . html_escape(get_request_var('drp_action')) . "'>
$save_html
</td>
</tr>";
Expand Down Expand Up @@ -665,10 +665,10 @@ function form_actions() {
print " <tr>
<td class='saveRow'>
<input type='hidden' name='action' value='actions'>
<input type='hidden' name='id' value='" . get_request_var('id') . "'>
<input type='hidden' name='id' value='" . html_escape(get_request_var('id')) . "'>
<input type='hidden' name='save_templates' value='1'>
<input type='hidden' name='selected_items' value='" . (isset($array) ? serialize($array) : '') . "'>
<input type='hidden' name='drp_action' value='" . get_request_var('drp_action') . "'>
<input type='hidden' name='drp_action' value='" . html_escape(get_request_var('drp_action')) . "'>
$save_html
</td>
</tr>";
Expand Down Expand Up @@ -743,10 +743,10 @@ function form_actions() {
print " <tr>
<td class='saveRow'>
<input type='hidden' name='action' value='actions'>
<input type='hidden' name='id' value='" . get_request_var('id') . "'>
<input type='hidden' name='id' value='" . html_escape(get_request_var('id')) . "'>
<input type='hidden' name='save_tholds' value='1'>
<input type='hidden' name='selected_items' value='" . (isset($array) ? serialize($array) : '') . "'>
<input type='hidden' name='drp_action' value='" . get_request_var('drp_action') . "'>
<input type='hidden' name='drp_action' value='" . html_escape(get_request_var('drp_action')) . "'>
$save_html
</td>
</tr>";
Expand Down Expand Up @@ -828,10 +828,10 @@ function form_actions() {
print "<tr>
<td class='saveRow'>
<input type='hidden' name='action' value='actions'>
<input type='hidden' name='id' value='" . get_request_var('id') . "'>
<input type='hidden' name='id' value='" . html_escape(get_request_var('id')) . "'>
<input type='hidden' name='save_associate' value='1'>
<input type='hidden' name='selected_items' value='" . (isset($array) ? serialize($array) : '') . "'>
<input type='hidden' name='drp_action' value='" . get_request_var('drp_action') . "'>
<input type='hidden' name='drp_action' value='" . html_escape(get_request_var('drp_action')) . "'>
$save_html
</td>
</tr>";
Expand Down Expand Up @@ -1138,7 +1138,7 @@ function hosts($header_label) {
<script type='text/javascript'>

function applyFilter() {
strURL = '?header=false&action=edit&id=<?php print get_request_var('id'); ?>'
strURL = '?header=false&action=edit&id=<?php print (int)get_filter_request_var('id'); ?>'
strURL += '&rows=' + $('#rows').val();
strURL += '&host_template_id=' + $('#host_template_id').val();
strURL += '&site_id=' + $('#site_id').val();
Expand All @@ -1148,12 +1148,12 @@ function applyFilter() {
}

function clearFilter() {
strURL = 'notify_lists.php?header=false&action=edit&id=<?php print get_request_var('id'); ?>&clear=true'
strURL = 'notify_lists.php?header=false&action=edit&id=<?php print (int)get_filter_request_var('id'); ?>&clear=true'
loadPageNoHeader(strURL);
}

$(function() {
$('#form_devices').submit(function(event) {
$('#form_devices').on('submit', function(event) {
event.preventDefault();
applyFilter();
});
Expand Down Expand Up @@ -1241,7 +1241,7 @@ function clearFilter() {

$hosts = db_fetch_assoc_prepared($sql_query, $sql_params);

$nav = html_nav_bar('notify_lists.php?action=edit&id=' . get_request_var('id'), MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, 10, __('Devices', 'thold'), 'page', 'main');
$nav = html_nav_bar('notify_lists.php?action=edit&id=' . (int)get_request_var('id'), MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, 10, __('Devices', 'thold'), 'page', 'main');

form_start('notify_lists.php', 'chk');

Expand Down Expand Up @@ -1387,27 +1387,27 @@ function tholds($header_label) {
$limit = ($rows * (intval(get_request_var('page')) - 1)) . ", $rows";

if (!isempty_request_var('template') && get_request_var('template') != '-1') {
$sql_where .= ($sql_where == '' ? '' : ' AND ') . 'td.data_template_id = ' . get_request_var('template');
$sql_where .= ($sql_where == '' ? '' : ' AND ') . 'td.data_template_id = ' . (int)get_request_var('template');
}

if (get_request_var('site_id') == '-1') {
// Show all items
} elseif (get_request_var('site_id') == '0') {
$sql_where .= ($sql_where == '' ? '' : ' AND ') . ' h.site_id=0';
} elseif (!isempty_request_var('site_id')) {
$sql_where .= ($sql_where == '' ? '' : ' AND ') . ' h.site_id=' . get_request_var('site_id');
$sql_where .= ($sql_where == '' ? '' : ' AND ') . ' h.site_id=' . (int)get_request_var('site_id');
}

if (strlen(get_request_var('rfilter'))) {
$sql_where .= (!strlen($sql_where) ? '' : ' AND ') . "td.name_cache RLIKE '" . get_request_var('rfilter') . "'";
$sql_where .= (!strlen($sql_where) ? '' : ' AND ') . 'td.name_cache RLIKE ' . db_qstr(get_request_var('rfilter'));
}

if ($statefilter != '') {
$sql_where .= (!strlen($sql_where) ? '' : ' AND ') . $statefilter;
}

if (get_request_var('associated') == 'true') {
$sql_where .= (!strlen($sql_where) ? '' : ' AND ') . '(td.notify_warning=' . get_request_var('id') . ' OR td.notify_alert=' . get_request_var('id') . ')';
$sql_where .= (!strlen($sql_where) ? '' : ' AND ') . '(td.notify_warning=' . (int)get_request_var('id') . ' OR td.notify_alert=' . (int)get_request_var('id') . ')';
}

$result = get_allowed_thresholds($sql_where, $sort, $limit, $total_rows);
Expand Down Expand Up @@ -1507,7 +1507,7 @@ function tholds($header_label) {
<script type='text/javascript'>

function applyFilter() {
strURL = 'notify_lists.php?header=false&action=edit&tab=tholds&id=<?php print get_request_var('id'); ?>'
strURL = 'notify_lists.php?header=false&action=edit&tab=tholds&id=<?php print (int)get_filter_request_var('id'); ?>'
strURL += '&associated=' + $('#associated').is(':checked');
strURL += '&state=' + $('#state').val();
strURL += '&site_id=' + $('#site_id').val();
Expand All @@ -1518,12 +1518,12 @@ function applyFilter() {
}

function clearFilter() {
strURL = 'notify_lists.php?header=false&action=edit&tab=tholds&id=<?php print get_request_var('id'); ?>&clear=true'
strURL = 'notify_lists.php?header=false&action=edit&tab=tholds&id=<?php print (int)get_filter_request_var('id'); ?>&clear=true'
loadPageNoHeader(strURL);
}

$(function() {
$('#listthold').submit(function(event) {
$('#listthold').on('submit', function(event) {
event.preventDefault();
applyFilter();
});
Expand Down Expand Up @@ -1796,20 +1796,20 @@ function templates($header_label) {
<script type='text/javascript'>

function applyFilter() {
strURL = 'notify_lists.php?header=false&action=edit&tab=templates&id=<?php print get_request_var('id'); ?>'
strURL = 'notify_lists.php?header=false&action=edit&tab=templates&id=<?php print (int)get_filter_request_var('id'); ?>'
strURL += '&associated=' + $('#associated').is(':checked');
strURL += '&rows=' + $('#rows').val();
strURL += '&rfilter=' + base64_encode($('#rfilter').val());
loadPageNoHeader(strURL);
}

function clearFilter() {
strURL = 'notify_lists.php?header=false&action=edit&tab=templates&id=<?php print get_request_var('id'); ?>&clear=true'
strURL = 'notify_lists.php?header=false&action=edit&tab=templates&id=<?php print (int)get_filter_request_var('id'); ?>&clear=true'
loadPageNoHeader(strURL);
}

$(function() {
$('#listthold').submit(function(event) {
$('#listthold').on('submit', function(event) {
event.preventDefault();
applyFilter();
});
Expand Down Expand Up @@ -2128,7 +2128,7 @@ function clearFilter() {
}

$(function() {
$('#lists').submit(function(event) {
$('#lists').on('submit', function(event) {
event.preventDefault();
applyFilter();
});
Expand Down
12 changes: 6 additions & 6 deletions notify_queue.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,30 +331,30 @@ function clearFilter() {
}

$(function() {
$('#refresh').click(function() {
$('#refresh').on('click', function() {
applyFilter();
});

$('#clear').click(function() {
$('#clear').on('click', function() {
clearFilter();
});

$('#suspend').click(function() {
$('#suspend').on('click', function() {
strURL = 'notify_queue.php?action=suspend';
loadPage(strURL);
});

$('#resume').click(function() {
$('#resume').on('click', function() {
strURL = 'notify_queue.php?action=resume';
loadPage(strURL);
});

$('#purge').click(function() {
$('#purge').on('click', function() {
strURL = 'notify_queue.php?action=purge';
loadPage(strURL);
});

$('#form_notify').submit(function(event) {
$('#form_notify').on('submit', function(event) {
event.preventDefault();
applyFilter();
});
Expand Down
34 changes: 16 additions & 18 deletions setup.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1233,7 +1233,7 @@ function thold_page_head() {
<script type='text/javascript'>
$(function() {
$(document).ajaxComplete(function() {
$('.tholdVRule').unbind().click(function(event) {
$('.tholdVRule').off().on('click', function(event) {
event.preventDefault();

href = $(this).attr('href');
Expand Down Expand Up @@ -1394,14 +1394,14 @@ function thold_device_top() {
$('#cdialog').dialog();
});

$('#continue').click(function(data) {
$('#continue').on('click', function(data) {
$.post('host.php?action=item_remove_tt', {
__csrf_magic: csrfMagicToken,
host_id: <?php print get_request_var('host_id'); ?>,
id: <?php print get_request_var('id'); ?>
host_id: <?php print (int)get_filter_request_var('host_id'); ?>,
id: <?php print (int)get_filter_request_var('id'); ?>
}).done(function(data) {
$('#cdialog').dialog('close');
loadPageNoHeader('host.php?action=edit&header=false&id=<?php print get_request_var('host_id'); ?>');
loadPageNoHeader('host.php?action=edit&header=false&id=<?php print (int)get_filter_request_var('host_id'); ?>');
});
});
</script>
Expand Down Expand Up @@ -1503,15 +1503,13 @@ function thold_device_template_edit() {
</table>
<script type='text/javascript'>
function addThresholdTemplate() {
$('#add_tt').click(function() {
scrollTop = $(window).scrollTop();
$.post('host_templates.php?header=false&action=item_add_tt', {
host_template_id: $('#id').val(),
thold_template_id: $('#thold_template_id').val(),
__csrf_magic: csrfMagicToken})
.done(function(data) {
loadPageNoHeader(urlPath+'host_templates.php?header=false&action=edit&id='+$('#id').val());
});
scrollTop = $(window).scrollTop();
$.post('host_templates.php?header=false&action=item_add_tt', {
host_template_id: $('#id').val(),
thold_template_id: $('#thold_template_id').val(),
__csrf_magic: csrfMagicToken})
.done(function(data) {
loadPageNoHeader(urlPath+'host_templates.php?header=false&action=edit&id='+$('#id').val());
});
}
</script>
Expand Down Expand Up @@ -1564,14 +1562,14 @@ function thold_device_template_top() {
$('#cdialog').dialog();
});

$('#continue').click(function(data) {
$('#continue').on('click', function(data) {
$.post('host_templates.php?action=item_remove_tt', {
__csrf_magic: csrfMagicToken,
host_template_id: <?php print get_request_var('host_template_id'); ?>,
id: <?php print get_request_var('id'); ?>
host_template_id: <?php print (int)get_filter_request_var('host_template_id'); ?>,
id: <?php print (int)get_filter_request_var('id'); ?>
}).done(function(data) {
$('#cdialog').dialog('close');
loadPageNoHeader('host_templates.php?action=edit&header=false&id=<?php print get_request_var('host_template_id'); ?>');
loadPageNoHeader('host_templates.php?action=edit&header=false&id=<?php print (int)get_filter_request_var('host_template_id'); ?>');
});
});
</script>
Expand Down
24 changes: 24 additions & 0 deletions tests/Integration/test_notify_list_wiring.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

$source = file_get_contents(dirname(__DIR__, 2) . '/notify_lists.php');

if ($source === false) {
fwrite(STDERR, "Unable to read notify_lists.php\n");
exit(1);
}

$checks = array(
"id=<?php print (int)get_filter_request_var('id'); ?>",
"'notify_lists.php?action=edit&id=' . (int)get_request_var('id')",
"<input type='hidden' name='id' value='\" . html_escape(get_request_var('id')) . \"'>",
"<input type='hidden' name='drp_action' value='\" . html_escape(get_request_var('drp_action')) . \"'>",
);

foreach ($checks as $needle) {
if (strpos($source, $needle) === false) {
fwrite(STDERR, "Missing expected notify list wiring\n");
exit(1);
}
}

echo "OK\n";
26 changes: 26 additions & 0 deletions tests/Unit/test_notify_list_security_guards.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

$source = file_get_contents(dirname(__DIR__, 2) . '/notify_lists.php');

if ($source === false) {
fwrite(STDERR, "Unable to read notify_lists.php\n");
exit(1);
}

$needles = array(
"html_escape(get_request_var('drp_action'))",
"html_escape(get_request_var('id'))",
"'td.data_template_id = ' . (int)get_request_var('template')",
"' h.site_id=' . (int)get_request_var('site_id')",
"'td.name_cache RLIKE ' . db_qstr(get_request_var('rfilter'))",
"'(td.notify_warning=' . (int)get_request_var('id') . ' OR td.notify_alert=' . (int)get_request_var('id') . ')'",
);

foreach ($needles as $needle) {
if (strpos($source, $needle) === false) {
fwrite(STDERR, "Missing expected notify list guard\n");
exit(1);
}
}

echo "OK\n";
26 changes: 26 additions & 0 deletions tests/e2e/test_notify_list_no_raw_sql_or_form_reuse.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

$source = file_get_contents(dirname(__DIR__, 2) . '/notify_lists.php');

if ($source === false) {
fwrite(STDERR, "Unable to read notify_lists.php\n");
exit(1);
}

$legacy = array(
"<input type='hidden' name='drp_action' value='\" . get_request_var('drp_action') . \"'>",
"<input type='hidden' name='id' value='\" . get_request_var('id') . \"'>",
"td.name_cache RLIKE '\" . get_request_var('rfilter') . \"'",
"'td.data_template_id = ' . get_request_var('template')",
"' h.site_id=' . get_request_var('site_id')",
"'(td.notify_warning=' . get_request_var('id') . ' OR td.notify_alert=' . get_request_var('id') . ')'",
);

foreach ($legacy as $needle) {
if (strpos($source, $needle) !== false) {
fwrite(STDERR, "Found legacy insecure notify list pattern\n");
exit(1);
}
}

echo "OK\n";
2 changes: 1 addition & 1 deletion thold.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ function clearFilter() {
}

$(function() {
$('#thold').submit(function(event) {
$('#thold').on('submit', function(event) {
event.preventDefault();
applyFilter();
});
Expand Down
Loading
Loading