Skip to content

Commit e591b10

Browse files
somethingwithproofsomethingwithproof
committed
fix(security): harden notify list form and filter handling
1 parent 7191dbc commit e591b10

4 files changed

Lines changed: 88 additions & 12 deletions

File tree

‎notify_lists.php‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,7 @@ function form_actions() {
590590
<input type='hidden' name='action' value='actions'>
591591
<input type='hidden' name='save_list' value='1'>
592592
<input type='hidden' name='selected_items' value='" . (isset($array) ? serialize($array) : '') . "'>
593-
<input type='hidden' name='drp_action' value='" . get_request_var('drp_action') . "'>
593+
<input type='hidden' name='drp_action' value='" . html_escape(get_request_var('drp_action')) . "'>
594594
$save_html
595595
</td>
596596
</tr>";
@@ -665,10 +665,10 @@ function form_actions() {
665665
print " <tr>
666666
<td class='saveRow'>
667667
<input type='hidden' name='action' value='actions'>
668-
<input type='hidden' name='id' value='" . get_request_var('id') . "'>
668+
<input type='hidden' name='id' value='" . html_escape(get_request_var('id')) . "'>
669669
<input type='hidden' name='save_templates' value='1'>
670670
<input type='hidden' name='selected_items' value='" . (isset($array) ? serialize($array) : '') . "'>
671-
<input type='hidden' name='drp_action' value='" . get_request_var('drp_action') . "'>
671+
<input type='hidden' name='drp_action' value='" . html_escape(get_request_var('drp_action')) . "'>
672672
$save_html
673673
</td>
674674
</tr>";
@@ -743,10 +743,10 @@ function form_actions() {
743743
print " <tr>
744744
<td class='saveRow'>
745745
<input type='hidden' name='action' value='actions'>
746-
<input type='hidden' name='id' value='" . get_request_var('id') . "'>
746+
<input type='hidden' name='id' value='" . html_escape(get_request_var('id')) . "'>
747747
<input type='hidden' name='save_tholds' value='1'>
748748
<input type='hidden' name='selected_items' value='" . (isset($array) ? serialize($array) : '') . "'>
749-
<input type='hidden' name='drp_action' value='" . get_request_var('drp_action') . "'>
749+
<input type='hidden' name='drp_action' value='" . html_escape(get_request_var('drp_action')) . "'>
750750
$save_html
751751
</td>
752752
</tr>";
@@ -828,10 +828,10 @@ function form_actions() {
828828
print "<tr>
829829
<td class='saveRow'>
830830
<input type='hidden' name='action' value='actions'>
831-
<input type='hidden' name='id' value='" . get_request_var('id') . "'>
831+
<input type='hidden' name='id' value='" . html_escape(get_request_var('id')) . "'>
832832
<input type='hidden' name='save_associate' value='1'>
833833
<input type='hidden' name='selected_items' value='" . (isset($array) ? serialize($array) : '') . "'>
834-
<input type='hidden' name='drp_action' value='" . get_request_var('drp_action') . "'>
834+
<input type='hidden' name='drp_action' value='" . html_escape(get_request_var('drp_action')) . "'>
835835
$save_html
836836
</td>
837837
</tr>";
@@ -1241,7 +1241,7 @@ function clearFilter() {
12411241

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

1244-
$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');
1244+
$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');
12451245

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

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

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

13931393
if (get_request_var('site_id') == '-1') {
13941394
// Show all items
13951395
} elseif (get_request_var('site_id') == '0') {
13961396
$sql_where .= ($sql_where == '' ? '' : ' AND ') . ' h.site_id=0';
13971397
} elseif (!isempty_request_var('site_id')) {
1398-
$sql_where .= ($sql_where == '' ? '' : ' AND ') . ' h.site_id=' . get_request_var('site_id');
1398+
$sql_where .= ($sql_where == '' ? '' : ' AND ') . ' h.site_id=' . (int)get_request_var('site_id');
13991399
}
14001400

14011401
if (strlen(get_request_var('rfilter'))) {
1402-
$sql_where .= (!strlen($sql_where) ? '' : ' AND ') . "td.name_cache RLIKE '" . get_request_var('rfilter') . "'";
1402+
$sql_where .= (!strlen($sql_where) ? '' : ' AND ') . 'td.name_cache RLIKE ' . db_qstr(get_request_var('rfilter'));
14031403
}
14041404

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

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

14131413
$result = get_allowed_thresholds($sql_where, $sort, $limit, $total_rows);

‎tests/Integration/test_notify_list_wiring.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
$source = file_get_contents(dirname(__DIR__, 2) . '/notify_lists.php');
4+
5+
if ($source === false) {
6+
fwrite(STDERR, "Unable to read notify_lists.php\n");
7+
exit(1);
8+
}
9+
10+
$checks = array(
11+
"id=<?php print (int)get_filter_request_var('id'); ?>",
12+
"'notify_lists.php?action=edit&id=' . (int)get_request_var('id')",
13+
"<input type='hidden' name='id' value='\" . html_escape(get_request_var('id')) . \"'>",
14+
"<input type='hidden' name='drp_action' value='\" . html_escape(get_request_var('drp_action')) . \"'>",
15+
);
16+
17+
foreach ($checks as $needle) {
18+
if (strpos($source, $needle) === false) {
19+
fwrite(STDERR, "Missing expected notify list wiring\n");
20+
exit(1);
21+
}
22+
}
23+
24+
echo "OK\n";

‎tests/Unit/test_notify_list_security_guards.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
$source = file_get_contents(dirname(__DIR__, 2) . '/notify_lists.php');
4+
5+
if ($source === false) {
6+
fwrite(STDERR, "Unable to read notify_lists.php\n");
7+
exit(1);
8+
}
9+
10+
$needles = array(
11+
"html_escape(get_request_var('drp_action'))",
12+
"html_escape(get_request_var('id'))",
13+
"'td.data_template_id = ' . (int)get_request_var('template')",
14+
"' h.site_id=' . (int)get_request_var('site_id')",
15+
"'td.name_cache RLIKE ' . db_qstr(get_request_var('rfilter'))",
16+
"'(td.notify_warning=' . (int)get_request_var('id') . ' OR td.notify_alert=' . (int)get_request_var('id') . ')'",
17+
);
18+
19+
foreach ($needles as $needle) {
20+
if (strpos($source, $needle) === false) {
21+
fwrite(STDERR, "Missing expected notify list guard\n");
22+
exit(1);
23+
}
24+
}
25+
26+
echo "OK\n";

‎tests/e2e/test_notify_list_no_raw_sql_or_form_reuse.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
$source = file_get_contents(dirname(__DIR__, 2) . '/notify_lists.php');
4+
5+
if ($source === false) {
6+
fwrite(STDERR, "Unable to read notify_lists.php\n");
7+
exit(1);
8+
}
9+
10+
$legacy = array(
11+
"<input type='hidden' name='drp_action' value='\" . get_request_var('drp_action') . \"'>",
12+
"<input type='hidden' name='id' value='\" . get_request_var('id') . \"'>",
13+
"td.name_cache RLIKE '\" . get_request_var('rfilter') . \"'",
14+
"'td.data_template_id = ' . get_request_var('template')",
15+
"' h.site_id=' . get_request_var('site_id')",
16+
"'(td.notify_warning=' . get_request_var('id') . ' OR td.notify_alert=' . get_request_var('id') . ')'",
17+
);
18+
19+
foreach ($legacy as $needle) {
20+
if (strpos($source, $needle) !== false) {
21+
fwrite(STDERR, "Found legacy insecure notify list pattern\n");
22+
exit(1);
23+
}
24+
}
25+
26+
echo "OK\n";

0 commit comments

Comments
 (0)