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 1 commit
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
12 changes: 6 additions & 6 deletions notify_lists.php
View file
Open in desktop
Original file line number Diff line number Diff line change
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,7 +1148,7 @@ 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);
}

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,7 +1518,7 @@ 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);
}

Expand Down Expand Up @@ -1796,15 +1796,15 @@ 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);
}

Expand Down
12 changes: 6 additions & 6 deletions setup.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1397,11 +1397,11 @@ function thold_device_top() {
$('#continue').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 @@ -1567,11 +1567,11 @@ function thold_device_template_top() {
$('#continue').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
4 changes: 2 additions & 2 deletions thold_graph.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ function form_thold_filter() {
</td>
</tr>
</table>
<input type='hidden' id='page' value='<?php print get_request_var('page'); ?>'>
<input type='hidden' id='page' value='<?php print html_escape_request_var('page'); ?>'>
<input type='hidden' id='tab' value='thold'>
</form>
<script type='text/javascript'>
Expand Down Expand Up @@ -1261,7 +1261,7 @@ function form_host_filter() {
</td>
</tr>
</table>
<input type='hidden' name='page' value='<?php print get_request_var('page'); ?>'>
<input type='hidden' name='page' value='<?php print html_escape_request_var('page'); ?>'>
<input type='hidden' name='tab' value='hoststat'>
</form>
<script type='text/javascript'>
Expand Down
2 changes: 1 addition & 1 deletion thold_process.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@
$item = [];

if (substr($thold_data['rrd_reindexed'], 0, 1) == 'a') {
$rrd_reindexed[$thold_data['local_data_id']] = cacti_unserialize($thold_data['rrd_reindexed']);
$rrd_reindexed[$thold_data['local_data_id']] = cacti_unserialize($thold_data['rrd_reindexed'], array('allowed_classes' => false));
} else {
$rrd_reindexed[$thold_data['local_data_id']] = json_decode($thold_data['rrd_reindexed'], true);
}
Expand Down
2 changes: 1 addition & 1 deletion thold_webapi.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,7 @@ function applyTholdFilter() {
function thold_new_graphs_save($host_id) {
$return_array = false;

$selected_graphs_array = cacti_unserialize(stripslashes(get_nfilter_request_var('selected_graphs_array')));
$selected_graphs_array = cacti_unserialize(stripslashes(get_nfilter_request_var('selected_graphs_array', array('allowed_classes' => false))));
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

get_nfilter_request_var() is being called with a second argument (array('allowed_classes' => false)), but this codebase otherwise only calls it with a single parameter. This is likely a runtime error (too many arguments) and also doesn’t apply the allowed_classes restriction to the unserialize operation. Pass the options array to cacti_unserialize() instead (second parameter), and keep get_nfilter_request_var('selected_graphs_array') as-is.

Suggested change
$selected_graphs_array = cacti_unserialize(stripslashes(get_nfilter_request_var('selected_graphs_array', array('allowed_classes' => false))));
$selected_graphs_array = cacti_unserialize(stripslashes(get_nfilter_request_var('selected_graphs_array')), array('allowed_classes' => false));

Copilot uses AI. Check for mistakes.

$values = [];

Expand Down
Loading