* security: require CSRF-protected POST for link and plugin reordering (#7454)
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix(csrf): share POST and token guards for reorder actions
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* Stop data_source_path_within_rra() flagging a not-yet-created intermediate directory as an RRA escape
get_data_source_path() routed a legitimate <path_rra>/<host_id>/<local_data_id>.rrd through the generated-path fallback and logged a bogus 'SECURITY: ... escapes the RRA directory' message whenever the host_id subdirectory did not exist yet (first poll, or host_id 0). data_source_path_within_rra() only tolerated a missing final RRD file: for a missing file it resolved realpath(dirname(path)), which returns false when that parent directory has not been created, so the lexically-contained path was wrongly rejected.
Walk to the deepest ancestor that actually exists and confirm it resolves inside the RRA root instead. Lexical containment (no .., '.', or empty segments) and per-segment symlink rejection are unchanged, so traversal, lookalike-prefix, absolute, and symlink-pivot paths are still rejected. Adds a regression test for a not-yet-created intermediate directory.
* Drop explanatory comments from the RRA containment fix
* Add CHANGELOG fragment for the RRA containment fix
* Reject a dangling-symlink segment independently of file_exists()
* test(paths): cover RRA filesystem boundaries and gate PR coverage
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* ci: measure committed PR changes on a policy-compliant runner
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: Thomas Vincent <thomasvincent@gmail.com>
* Fix false-positive RRA-escape warning when path_rra has a trailing slash
A path_rra setting saved with a trailing slash makes CACTI_PATH_RRA end in
'/', so the '<path_rra>/' -> CACTI_PATH_RRA . '/' expansion that every data
source consumer performs yields a doubled slash ('<rra>//0/1457.rrd').
data_source_path_within_rra() split the expanded path on '/' and rejected any
empty segment, so the doubled slash was read as a traversal and a perfectly
legitimate RRD file was reported as escaping the RRA directory:
SECURITY: Data source 1457 has a data_source_path that escapes the RRA
directory (<path_rra>/0/1457.rrd). Using the generated path instead.
get_data_source_path() then returned the generated fallback path, which the
poller had never created, producing the follow-on 'RRD file does not exist'
graph warnings (note the tell-tale 'rra//0' doubled slash in those lines).
Collapse repeated slashes and strip a trailing slash from the RRA root before
the containment comparison. A '//' is an empty segment, not a parent
reference, so normalising it keeps the file contained while '..' traversal,
absolute paths elsewhere, lookalike prefixes and symlink pivots are still
rejected (asserted by the expanded regression test).
* Keep data_source_path_within_rra() normalisation strictly string-typed
preg_replace() is typed array<string>|string|null, so feeding its result to
rtrim()/strlen() tripped PHPStan (level 6/8). Cast CACTI_PATH_RRA and both
preg_replace() results to string; runtime behaviour is unchanged.
* Cover the trailing-slash CACTI_PATH_RRA half of the fix in an isolated process
The existing cases insert '//' into the target only; data_source_path_test_base()
uses realpath(), which strips the trailing slash, so they never exercise the
rtrim() on \. CACTI_PATH_RRA is immutable, so define it with a trailing
slash in a fresh PHP process and assert a clean/doubled-slash file stays
contained while a traversal is still rejected.
---------
Co-authored-by: TheWitness <1439914+TheWitness@users.noreply.github.com>
* security: contain data source paths within the RRA directory (GHSA-9qff-xccr-rrc5)
* security: escape executable path in exec_background on non-win32 (GHSA-cv7j-p6fh-j4q7)
* security: stop disabled accounts authenticating via remember-me cookie and revoke on disable (GHSA-pqrp-v752-mc4f)
* security: prefer base_url over client Host for the force_https redirect (GHSA-3466-fp7g-c9rf)
* changelog: add fragments for GHSA-9qff-xccr-rrc5, GHSA-cv7j-p6fh-j4q7, GHSA-pqrp-v752-mc4f, GHSA-3466-fp7g-c9rf
* security: address review findings on GHSA batch 2 (develop)
- lib/dsstats.php, poller_maintenance.php: route through
get_data_source_path() instead of expanding data_source_path directly,
so the RRA containment check applies to these consumers too (GHSA-9qff)
- lib/functions.php: data_source_path_within_rra() now also validates
the realpath of each existing ancestor segment, closing a symlink
pivot that the purely lexical check missed (GHSA-9qff)
- include/global.php: fail closed instead of falling back to SERVER_NAME
when base_url is unset/invalid, removing the residual open-redirect
path (GHSA-3466)
- lib/html_utility.php: cacti_force_https_host()/
cacti_build_https_redirect_url() now parse a scheme-less base_url
(matching lib/reports.php's own normalization) and preserve a
configured non-default port instead of silently dropping it (GHSA-3466)
- tests: move DataSourcePathContainmentTest.php into
tests/Unit/Security/PathTraversal/, and switch its fixtures from a
fictional CACTI_PATH_RRA path to a real temp directory so the new
realpath()-based checks (including a symlink-pivot regression case)
can actually run
* test: fix CACTI_PATH_RRA constant collision in DataSourcePathContainmentTest
CI showed 2 failures: RrdProxyProtocolTest.php runs earlier in the same
Pest process and already defines CACTI_PATH_RRA to a fictional,
non-existent '/var/lib/cacti/rra', so our own definition guard never
ran and realpath(CACTI_PATH_RRA) returned false for every assertion,
failing the two tests that expect containment to be true.
Now ensure the directory backing CACTI_PATH_RRA actually exists on
disk regardless of who defined the constant first, and gracefully
skip only the fixture-dependent assertions if this process can't
create it (e.g. no permission on a system path).
* test: refresh sink inventory baseline for exec_background rename
5fbe90fb4 renamed \ to \ in lib/poller.php's
exec() calls (GHSA-cv7j-p6fh-j4q7) but never regenerated the tracked
sink inventory, so the 'sink and architectural hotspot guard' CI check
has been failing on this branch since that commit. Regenerated via
tests/security/build_sink_inventory.sh | tr -d '\r' | LC_ALL=C sort;
diff confirms only those two lines changed.
* fix: address CI failures on GHSA batch 2 (develop)
- lib/auth.php: verify the remember-me token before revoking a disabled
account's cache row, so a forged token no longer triggers a database
write (fixes AuthSystemRegressionIntegrationTest)
- apply php-cs-fixer coding standard fixes (comment style, phpdoc
alignment, list-to-short-array syntax) flagged by the locked
integration check
- refresh the architectural hotspots baseline for the exec_background
safe_filename rename
* fix: update PHPStan baseline for reordered check_auth_cookie and get_data_source_path
- lib/auth.php: reordering the disabled-account check after the token
match moved one 'enabled' offset access to a new line, bumping the
file's expected occurrence count from 3 to 4
- lib/functions.php: the GHSA-9qff-xccr-rrc5 containment log line reads
\['data_source_path'], a genuinely new array|bool access
not previously tracked
---------
Co-authored-by: copilot-swe-agent <copilot-swe-agent[bot]@users.noreply.github.com>
Thomas Vincent