Skip to content

Commit f81e911

Browse files
backport: consolidated security hardening and bugfixes for main
1 parent 9166406 commit f81e911

10 files changed

Lines changed: 322 additions & 17 deletions

CHANGELOG.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33
--- develop ---
44

55
* issue#250: Fix date filter persistence by validating before shift_span detection
6-
* issue#258: Execute CREATE TABLE SQL correctly during replication sync
7-
* issue#278: Extract duplicated alert command execution paths in syslog_process_alerts
8-
* issue#278: Extract alert command execution into shared helper in functions.php; command tokenization now uses preg_split (handles tabs and consecutive spaces); /bin/sh fallback for non-executable command templates removed (use absolute paths with execute bit set)
6+
* issue#260: Replace eval-based callback execution in autocomplete handling
97
* issue: Making changes to support Cacti 1.3
108
* issue: Don't use MyISAM for non-analytical tables
119
* issue: The install advisor for Syslog was broken in current Cacti releases

js/functions.js

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,11 @@ function initSyslogMain(config) {
220220

221221
$.each(data, function(index, hostData) {
222222
if ($('#host option[value="'+index+'"]').length == 0) {
223-
$('#host').append('<option class="'+hostData.class+'" value="'+index+'">'+hostData.host+'</option>');
223+
var option = $('<option>')
224+
.addClass(hostData.class || '')
225+
.val(index)
226+
.text(hostData.host);
227+
$('#host').append(option);
224228
}
225229
});
226230

@@ -567,6 +571,25 @@ function initSyslogReports() {
567571
* Autocomplete Form Callback Functions
568572
* ======================================================================== */
569573

574+
/**
575+
* Validate and invoke a named callback function specified as a string
576+
* @param {string} onChange - Name of the global function to call (e.g. 'myCallback')
577+
*/
578+
function runSyslogAutocompleteOnChange(onChange) {
579+
if (typeof onChange !== 'string') {
580+
return;
581+
}
582+
583+
var callbackName = onChange.trim().replace(/\(\)\s*$/, '');
584+
if (!callbackName.match(/^[A-Za-z_$][A-Za-z0-9_$]*$/)) {
585+
return;
586+
}
587+
588+
if (typeof window[callbackName] === 'function') {
589+
window[callbackName]();
590+
}
591+
}
592+
570593
/**
571594
* Initialize autocomplete for form dropdown fields
572595
* @param {string} formName - The name of the form field
@@ -591,7 +614,7 @@ function initSyslogAutocomplete(formName, callback, onChange) {
591614
$('#' + formName).val(ui.item.value);
592615
}
593616
if (onChange) {
594-
eval(onChange);
617+
runSyslogAutocompleteOnChange(onChange);
595618
}
596619
}
597620
}).css('border', 'none').css('background-color', 'transparent');

setup.php

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1573,6 +1573,25 @@ function syslog_utilities_action($action) {
15731573
}
15741574

15751575
if ($action == 'purge_syslog_hosts') {
1576+
if ($_SERVER['REQUEST_METHOD'] !== 'POST') {
1577+
raise_message('syslog_error', __('Invalid request. This action requires a CSRF protected POST.', 'syslog'), MESSAGE_LEVEL_ERROR);
1578+
header('Location: utilities.php');
1579+
exit;
1580+
}
1581+
1582+
if (function_exists('csrf_check')) {
1583+
if (!csrf_check(false)) {
1584+
raise_message('syslog_error', __('Invalid request. This action requires a CSRF protected POST.', 'syslog'), MESSAGE_LEVEL_ERROR);
1585+
header('Location: utilities.php');
1586+
exit;
1587+
}
1588+
} else {
1589+
cacti_log('WARNING: syslog purge blocked -- CSRF validation unavailable', false, 'SYSLOG');
1590+
raise_message('syslog_error', __('Invalid request. Please try again.', 'syslog'), MESSAGE_LEVEL_ERROR);
1591+
header('Location: utilities.php');
1592+
exit;
1593+
}
1594+
15761595
$records = 0;
15771596

15781597
syslog_db_execute('DELETE FROM syslog_hosts
@@ -1625,7 +1644,50 @@ function syslog_utilities_list() {
16251644

16261645
<tr class='even'>
16271646
<td>
1628-
<a class='hyperLink' href='utilities.php?action=purge_syslog_hosts'><?php print __('Purge Syslog Devices', 'syslog');?></a>
1647+
<input id='syslog_purge_hosts' type='button' value='<?php print __esc('Purge Syslog Devices', 'syslog');?>'>
1648+
<div id='syslog_purge_dialog' style='display:none;'>
1649+
<p><?php print __esc('Are you sure you want to purge stale Syslog devices?', 'syslog');?></p>
1650+
</div>
1651+
<script type='text/javascript'>
1652+
$(function() {
1653+
$('#syslog_purge_hosts').on('click', function() {
1654+
$('#syslog_purge_dialog').dialog({
1655+
title: <?php print json_encode(__('Confirm Purge', 'syslog'), JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT);?>,
1656+
minHeight: 80,
1657+
minWidth: 400,
1658+
resizable: false,
1659+
draggable: true,
1660+
buttons: {
1661+
'Cancel': {
1662+
text: <?php print json_encode(__('Cancel', 'syslog'), JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT);?>,
1663+
id: 'btnPurgeCancel',
1664+
click: function() {
1665+
$(this).dialog('close');
1666+
}
1667+
},
1668+
'Continue': {
1669+
text: <?php print json_encode(__('Continue', 'syslog'), JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT);?>,
1670+
id: 'btnPurgeContinue',
1671+
click: function() {
1672+
$(this).dialog('close');
1673+
1674+
var postData = {
1675+
action: 'purge_syslog_hosts',
1676+
__csrf_magic: csrfMagicToken
1677+
};
1678+
1679+
if (typeof postUrl == 'function') {
1680+
postUrl({url: 'utilities.php', noState: true}, postData);
1681+
} else {
1682+
loadPageUsingPost('utilities.php', postData);
1683+
}
1684+
}
1685+
}
1686+
}
1687+
});
1688+
});
1689+
});
1690+
</script>
16291691
</td>
16301692
<td>
16311693
<?php print __('This menu pick provides a means to remove Devices that are no longer reporting into Cacti\'s syslog server.', 'syslog');?>

syslog.php

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -364,10 +364,10 @@ function syslog_statistics() {
364364

365365
form_alternate_row('line' . $i);
366366

367-
print '<td>' . (get_request_var('host') != '-2' ? $r['host']:'-') . '</td>';
368-
print '<td>' . (get_request_var('facility') != '-2' ? ucfirst($r['facility']):'-') . '</td>';
369-
print '<td>' . (get_request_var('priority') != '-2' ? ucfirst($r['priority']):'-') . '</td>';
370-
print '<td>' . (get_request_var('program') != '-2' ? ucfirst($r['program']):'-') . '</td>';
367+
print '<td>' . (get_request_var('host') != '-2' ? html_escape($r['host']):'-') . '</td>';
368+
print '<td>' . (get_request_var('facility') != '-2' ? html_escape(ucfirst($r['facility'])):'-') . '</td>';
369+
print '<td>' . (get_request_var('priority') != '-2' ? html_escape(ucfirst($r['priority'])):'-') . '</td>';
370+
print '<td>' . (get_request_var('program') != '-2' ? html_escape(ucfirst($r['program'])):'-') . '</td>';
371371
//print '<td class="right">' . $r['insert_time'] . '</td>';
372372
print '<td class="right">' . $time . '</td>';
373373
print '<td class="right">' . number_format_i18n($r['records'], -1) . '</td>';
@@ -521,7 +521,7 @@ function syslog_stats_filter() {
521521
$class = 'deviceUp';
522522
}
523523

524-
print '<option class="' . $class . '" value="' . $host['host_id'] . '"'; if (get_request_var('host') == $host['host_id']) { print ' selected'; } print '>' . $host['host'] . '</option>';
524+
print '<option class="' . $class . '" value="' . $host['host_id'] . '"'; if (get_request_var('host') == $host['host_id']) { print ' selected'; } print '>' . html_escape($host['host']) . '</option>';
525525
}
526526
}
527527
?>
@@ -541,7 +541,7 @@ function syslog_stats_filter() {
541541

542542
if (cacti_sizeof($facilities)) {
543543
foreach ($facilities as $r) {
544-
print '<option value="' . $r['facility_id'] . '"'; if (get_request_var('facility') == $r['facility_id']) { print ' selected'; } print '>' . ucfirst($r['facility']) . "</option>\n";
544+
print '<option value="' . $r['facility_id'] . '"'; if (get_request_var('facility') == $r['facility_id']) { print ' selected'; } print '>' . html_escape(ucfirst($r['facility'])) . "</option>\n";
545545
}
546546
}
547547
?>
@@ -561,7 +561,7 @@ function syslog_stats_filter() {
561561

562562
if (cacti_sizeof($priorities)) {
563563
foreach ($priorities as $r) {
564-
print '<option value="' . $r['priority_id'] . '"'; if (get_request_var('priority') == $r['priority_id']) { print ' selected'; } print '>' . ucfirst($r['priority']) . "</option>\n";
564+
print '<option value="' . $r['priority_id'] . '"'; if (get_request_var('priority') == $r['priority_id']) { print ' selected'; } print '>' . html_escape(ucfirst($r['priority'])) . "</option>\n";
565565
}
566566
}
567567
?>
@@ -1367,7 +1367,7 @@ function syslog_filter($sql_where, $tab) {
13671367
}
13681368
}
13691369
print '>';
1370-
print $host['host'] . '</option>';
1370+
print html_escape($host['host']) . '</option>';
13711371
}
13721372
}
13731373
?>
@@ -2055,4 +2055,3 @@ function syslog_form_callback($form_name, $classic_sql, $column_display, $column
20552055
<?php
20562056
}
20572057
}
2058-

syslog_removal.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ function form_actions() {
169169
FROM `" . $syslogdb_default . "`.`syslog_remove`
170170
WHERE id=" . $matches[1]);
171171

172-
$removal_list .= '<li>' . $removal_info . '</li>';
172+
$removal_list .= '<li>' . html_escape($removal_info) . '</li>';
173173
$removal_array[] = $matches[1];
174174
}
175175
}

syslog_reports.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ function form_actions() {
157157
FROM `' . $syslogdb_default . '`.`syslog_reports`
158158
WHERE id=' . $matches[1]);
159159

160-
$report_list .= '<li>' . $report_info . '</li>';
160+
$report_list .= '<li>' . html_escape($report_info) . '</li>';
161161
$report_array[] = $matches[1];
162162
}
163163
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
$syslogPhp = file_get_contents(dirname(__DIR__, 2) . '/syslog.php');
4+
$reportsPhp = file_get_contents(dirname(__DIR__, 2) . '/syslog_reports.php');
5+
$removalPhp = file_get_contents(dirname(__DIR__, 2) . '/syslog_removal.php');
6+
$functionsJs = file_get_contents(dirname(__DIR__, 2) . '/js/functions.js');
7+
8+
if ($syslogPhp === false || $reportsPhp === false || $removalPhp === false || $functionsJs === false) {
9+
fwrite(STDERR, "Failed to load one or more plugin files for issue252 checks.\n");
10+
exit(1);
11+
}
12+
13+
if (substr_count($syslogPhp, "html_escape(\$host['host'])") < 2) {
14+
fwrite(STDERR, "Expected escaped host rendering in syslog.php output paths.\n");
15+
exit(1);
16+
}
17+
18+
if (strpos($reportsPhp, "html_escape(\$report_info)") === false) {
19+
fwrite(STDERR, "Expected escaped report confirmation list entries.\n");
20+
exit(1);
21+
}
22+
23+
if (strpos($removalPhp, "html_escape(\$removal_info)") === false) {
24+
fwrite(STDERR, "Expected escaped removal confirmation list entries.\n");
25+
exit(1);
26+
}
27+
28+
if (strpos($reportsPhp, "form_selectable_ecell(\$report['message'], \$report['id']);") === false) {
29+
fwrite(STDERR, "Expected escaped report message cell rendering.\n");
30+
exit(1);
31+
}
32+
33+
if (strpos($functionsJs, "var option = $('<option>')") === false ||
34+
strpos($functionsJs, ".text(hostData.host);") === false ||
35+
strpos($functionsJs, "$('#host').append(option);") === false) {
36+
fwrite(STDERR, "Expected DOM-safe host option rendering in js/functions.js.\n");
37+
exit(1);
38+
}
39+
40+
if (strpos($functionsJs, "$('#host').append('<option class=\"'+hostData.class+'\" value=\"'+index+'\">'+hostData.host+'</option>');") !== false) {
41+
fwrite(STDERR, "Legacy unsafe host option HTML concatenation still present.\n");
42+
exit(1);
43+
}
44+
45+
echo "issue252_xss_output_test passed\n";
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
<?php
2+
3+
$setup = file_get_contents(dirname(__DIR__, 2) . '/setup.php');
4+
5+
if ($setup === false) {
6+
fwrite(STDERR, "Failed to read setup.php\n");
7+
exit(1);
8+
}
9+
10+
$required = array(
11+
"if (\$_SERVER['REQUEST_METHOD'] !== 'POST')",
12+
"if (function_exists('csrf_check'))",
13+
"if (!csrf_check(false))",
14+
"__csrf_magic: csrfMagicToken"
15+
);
16+
17+
foreach ($required as $snippet) {
18+
if (strpos($setup, $snippet) === false) {
19+
fwrite(STDERR, "Missing expected CSRF hardening snippet: $snippet\n");
20+
exit(1);
21+
}
22+
}
23+
24+
if (strpos($setup, "href='utilities.php?action=purge_syslog_hosts'") !== false) {
25+
fwrite(STDERR, "Legacy GET purge link still present.\n");
26+
exit(1);
27+
}
28+
29+
// Verify fail-closed: the else branch (csrf_check unavailable) must reject, not fall through.
30+
// Assert globally safe properties rather than parsing the else block via brittle regex.
31+
32+
// The fallback path must not attempt manual token checking
33+
if (strpos($setup, "\$_POST['__csrf_magic']") !== false) {
34+
fwrite(STDERR, "Fallback CSRF branch must not check token presence; must fail closed.\n");
35+
exit(1);
36+
}
37+
38+
// The fallback path must log the blocked attempt
39+
if (strpos($setup, "cacti_log('WARNING: syslog purge blocked") === false) {
40+
fwrite(STDERR, "Fail-closed branch must call cacti_log() to audit blocked purge attempts.\n");
41+
exit(1);
42+
}
43+
44+
// Log message must name the specific failure reason for incident response
45+
if (strpos($setup, 'CSRF validation unavailable') === false) {
46+
fwrite(STDERR, "Log message must specify 'CSRF validation unavailable' for operational clarity.\n");
47+
exit(1);
48+
}
49+
50+
// Verify JS confirm() uses json_encode, not __esc() inside JS string
51+
if (preg_match("/confirm\(\s*'/", $setup)) {
52+
fwrite(STDERR, "JS confirm() must use json_encode() for safe encoding, not __esc() in a quoted string.\n");
53+
exit(1);
54+
}
55+
56+
if (strpos($setup, 'json_encode(__(') === false) {
57+
fwrite(STDERR, "Expected json_encode(__(...)) for JS-safe encoding of confirm message.\n");
58+
exit(1);
59+
}
60+
61+
// Verify json_encode uses JSON_HEX_TAG to prevent </script> breakout in HTML script context
62+
if (strpos($setup, 'JSON_HEX_TAG') === false) {
63+
fwrite(STDERR, "json_encode() must use JSON_HEX_TAG to prevent script-context breakout.\n");
64+
exit(1);
65+
}
66+
67+
if (strpos($setup, 'JSON_HEX_AMP') === false) {
68+
fwrite(STDERR, "json_encode() must use JSON_HEX_AMP to escape ampersands in script context.\n");
69+
exit(1);
70+
}
71+
72+
if (strpos($setup, 'JSON_HEX_APOS') === false) {
73+
fwrite(STDERR, "json_encode() must use JSON_HEX_APOS.\n");
74+
exit(1);
75+
}
76+
77+
if (strpos($setup, 'JSON_HEX_QUOT') === false) {
78+
fwrite(STDERR, "json_encode() must use JSON_HEX_QUOT.\n");
79+
exit(1);
80+
}
81+
82+
// Verify user-facing message does not expose CSRF internals (log message may use "CSRF")
83+
if (strpos($setup, "raise_message('syslog_error', __('CSRF") !== false) {
84+
fwrite(STDERR, "User-facing raise_message must not expose CSRF internals to end users.\n");
85+
exit(1);
86+
}
87+
88+
// Verify generic user-facing message is present
89+
if (strpos($setup, "Invalid request. Please try again.") === false) {
90+
fwrite(STDERR, "Fail-closed branch must use generic 'Invalid request. Please try again.' message.\n");
91+
exit(1);
92+
}
93+
94+
// Verify fail-closed raise_message uses MESSAGE_LEVEL_ERROR severity
95+
if (strpos($setup, "raise_message('syslog_error', __('Invalid request. Please try again.', 'syslog'), MESSAGE_LEVEL_ERROR)") === false) {
96+
fwrite(STDERR, "Fail-closed branch raise_message must use MESSAGE_LEVEL_ERROR severity.\n");
97+
exit(1);
98+
}
99+
100+
// Verify log message does not expose internal function name
101+
if (strpos($setup, 'csrf_check() unavailable') !== false) {
102+
fwrite(STDERR, "Log message must not name internal validation function.\n");
103+
exit(1);
104+
}
105+
106+
echo "issue259_csrf_purge_test passed\n";

0 commit comments

Comments
 (0)