hardening: enforce POST+CSRF for purge syslog devices utility#265
hardening: enforce POST+CSRF for purge syslog devices utility#265somethingwithproof wants to merge 2 commits intoCacti:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Hardens the Syslog plugin’s “Purge Syslog Devices” utility by requiring the action to be executed via a POST flow intended to include CSRF protection, rather than a direct GET link, preserving the existing redirect/message behavior after execution.
Changes:
- Added a server-side POST +
__csrf_magicpresence check before runningpurge_syslog_hosts. - Replaced the utilities UI GET link with a button-driven JS POST flow that submits
action=purge_syslog_hostsplus CSRF token data. - Kept the post-action redirect back to
utilities.phpwith UI messaging.
setup.php
Outdated
| if ($_SERVER['REQUEST_METHOD'] !== 'POST' || !isset($_POST['__csrf_magic'])) { | ||
| raise_message('syslog_error', __('Invalid request. This action requires a CSRF protected POST.', 'syslog'), MESSAGE_LEVEL_ERROR); | ||
| header('Location: utilities.php'); |
There was a problem hiding this comment.
PR metadata says this change “Fixes #255”, but issue #255 is about SQL concatenation in the legacy syslog_manage_items() batch path (e.g., message LIKE '%...%' / type='sql' clauses in functions.php). This PR only hardens the utilities purge action flow, so it doesn’t appear to address the referenced issue as described. Either update the PR description/linked issue, or include the SQL hardening changes for #255 in this PR.
setup.php
Outdated
| if ($_SERVER['REQUEST_METHOD'] !== 'POST' || !isset($_POST['__csrf_magic'])) { | ||
| raise_message('syslog_error', __('Invalid request. This action requires a CSRF protected POST.', 'syslog'), MESSAGE_LEVEL_ERROR); | ||
| header('Location: utilities.php'); | ||
| exit; | ||
| } |
There was a problem hiding this comment.
The current CSRF gate only checks for the presence of __csrf_magic and does not validate the token value. If CSRF-magic isn’t already enforcing server-side validation for this endpoint, an attacker could supply an arbitrary __csrf_magic value and still pass this check. Prefer invoking the standard Cacti CSRF validation mechanism for POST actions (or otherwise explicitly validate the token), while keeping the POST-only requirement.
There was a problem hiding this comment.
Primary validation uses csrf_check(false) when available; the __csrf_magic presence check is the fallback for environments without the csrf helper.
|
Addressed review feedback in the latest commit set:
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
somethingwithproof
force-pushed
the
hardening/csrf-purge-utility
branch
from
5c69e61 to
a458c87
Compare
Summary
purge_syslog_hostsutility actioncsrf_check(false)(with__csrf_magicfallback if csrf helper is unavailable)__csrf_magictests/regression/*.phpexistValidation
Fixes #259