Skip to content

Commit f595568

Browse files
somethingwithproofsomethingwithproof
committed
hardening: enforce POST+CSRF for purge syslog devices utility
Refs #259 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
1 parent 6902d91 commit f595568

2 files changed

Lines changed: 169 additions & 2 deletions

File tree

‎setup.php‎

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1566,6 +1566,25 @@ function syslog_utilities_action($action) {
15661566
}
15671567

15681568
if ($action == 'purge_syslog_hosts') {
1569+
if ($_SERVER['REQUEST_METHOD'] !== 'POST') {
1570+
raise_message('syslog_error', __('Invalid request. This action requires a CSRF protected POST.', 'syslog'), MESSAGE_LEVEL_ERROR);
1571+
header('Location: utilities.php');
1572+
exit;
1573+
}
1574+
1575+
if (function_exists('csrf_check')) {
1576+
if (!csrf_check(false)) {
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+
} else {
1582+
cacti_log('WARNING: syslog purge blocked -- CSRF validation unavailable', false, 'SYSLOG');
1583+
raise_message('syslog_error', __('Invalid request. Please try again.', 'syslog'), MESSAGE_LEVEL_ERROR);
1584+
header('Location: utilities.php');
1585+
exit;
1586+
}
1587+
15691588
$records = 0;
15701589

15711590
syslog_db_execute('DELETE FROM syslog_hosts
@@ -1618,12 +1637,54 @@ function syslog_utilities_list() {
16181637

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

‎tests/regression/issue259_csrf_purge_test.php‎

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)