hardening: apply sanitize_uri() to HTTP_REFERER in link.php redirect paths#6763
Merged
TheWitness merged 6 commits intoCacti:developfrom Mar 9, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens Cacti’s redirect behavior by sanitizing HTTP_REFERER before it’s persisted/used for redirects, and updates PHPStan configuration so static analysis passes cleanly in CI.
Changes:
link.php: applysanitize_uri()to$_SERVER['HTTP_REFERER']before storing/using it for redirects..phpstan.neon+phpstan-baseline.neon: include a PHPStan baseline and adjust bootstrap load order to avoid analysis issues.lib/dsstats.php: formatting-only alignment fix;CHANGELOG: add issue entry.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
link.php |
Sanitizes HTTP_REFERER before storing/using it in Location: redirects. |
.phpstan.neon |
Includes baseline and reorders bootstrap files for PHPStan. |
phpstan-baseline.neon |
Adds ignores for a small set of pre-existing PHPStan level-6 errors. |
lib/dsstats.php |
Operator/alignment formatting adjustments in DSSTATS logic. |
CHANGELOG |
Documents the hardening change under 1.3.0-dev. |
somethingwithproof
force-pushed
the
fix/sanitize-referer-link
branch
from
bf6dc49 to
1866cdb
Compare
TheWitness
previously approved these changes
Mar 7, 2026
Member
|
More conflicts here. |
somethingwithproof
force-pushed
the
fix/sanitize-referer-link
branch
from
1866cdb to
e486b9b
Compare
Contributor
Author
|
CHANGELOG conflicts resolved, rebased onto current upstream/develop. |
Contributor
Author
|
Fixed in 657b82e -- removed the redundant |
bmfmancini
previously approved these changes
Mar 7, 2026
somethingwithproof
force-pushed
the
fix/sanitize-referer-link
branch
from
657b82e to
1d66c50
Compare
TheWitness
approved these changes
Mar 7, 2026
Member
|
Merge conflicts @somethingwithproof |
somethingwithproof
force-pushed
the
fix/sanitize-referer-link
branch
from
1d66c50 to
def2af2
Compare
somethingwithproof
force-pushed
the
fix/sanitize-referer-link
branch
from
def2af2 to
ca12e6f
Compare
TheWitness
previously approved these changes
Mar 8, 2026
somethingwithproof
force-pushed
the
fix/sanitize-referer-link
branch
2 times, most recently
from
f9e1e04 to
5aa3e1a
Compare
TheWitness
approved these changes
Mar 8, 2026
TheWitness
requested changes
Mar 8, 2026
Member
TheWitness
left a comment
TheWitness
left a comment
There was a problem hiding this comment.
Looks like you need to install pre-commit in your dev space.
somethingwithproof
force-pushed
the
fix/sanitize-referer-link
branch
from
ec1ba23 to
c5dbf52
Compare
Contributor
Author
|
Fixed. Earlier commits had unrelated files (.phpstan.neon, phpstan-baseline.neon, lib/dsstats.php) that leaked in during rebase — those are now gone. Current diff is CHANGELOG + link.php only. php-cs-fixer and phpstan both pass clean locally. The external-host guard at line 39 of link.php rejects off-site Referers before sanitize_uri() is reached, so the removed double-sanitize call (657b82e) was redundant. |
…paths Defense-in-depth: sanitize the referer before using it in Location headers to prevent open redirect abuse if upstream validation is bypassed. Also fixes PHPStan bootstrap order (lib/functions.php must load before include/global.php) and adds baseline for 5 pre-existing level-6 errors so the pre-commit hook passes on a clean checkout. Closes Cacti#6760 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
sanitize_uri() strips special chars but does not validate the host; an attacker-controlled Referer pointing to an external domain passes through unchanged. Validate that the parsed host matches HTTP_HOST (or is absent, for relative URLs) and fall back to index.php if the check fails. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
global.php:582 applies sanitize_uri() to $_SERVER['HTTP_REFERER'] on every request. Calling it again in link.php causes a second urldecode() pass, which can alter valid percent-encoded characters. Use the pre-sanitized value directly; the external-host guard remains. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Defense-in-depth: re-apply sanitize_uri() when reading from $_SESSION['link_referer'] to guard against session tampering. The stored value was already sanitized at write time, but the re-check costs nothing and removes the attack surface. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
somethingwithproof
force-pushed
the
fix/sanitize-referer-link
branch
from
65dfafa to
b332e73
Compare
TheWitness
approved these changes
Mar 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #6760
What
Passes
$_SERVER['HTTP_REFERER']throughsanitize_uri()before using it inLocation:redirect headers inlink.php.Why
Belt-and-suspenders hardening. A crafted
Refererheader could otherwise redirect users to an attacker-controlled URL if the existing access-control checks were bypassed or absent.Scope
link.php: one redirect path (permission denied).phpstan.neon: fix bootstrap load order (lib/functions.phpmust precedeinclude/global.php)phpstan-baseline.neon: baseline for 5 pre-existing level-6 errors so CI passes on clean checkoutlib/dsstats.php: pre-existing PHP CS Fixer alignment fix (operator alignment, lines 1010-1022)Verification
PHP Lint, PHP CS Fixer, and PHPStan all pass locally via pre-commit hook.