fix: Simplify redirect handling in Cacti and Fix Multi-Sort (#6955) (#6978)

* fix: Simplify redirect handling in Cacti and Fix Multi-Sort (#6955)

* fix: Simplify redirect handling in Cacti

* fix: Slight improvements from Copilot review

* fix: Fix for compatibility

* fix: Fix small syntax error

* fix: Remove comments

* fix: Make the query case insensitive

* fix: Encode the reference url

* fix: More copilot review comments

* fix: More copilot comments

* fix: Double semicolon

* fix: Missed one commit

* fix: Encode the correct url

* Update lib/html_utility.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* fix: Add additional checks

* fix: Fix order string updates for multiple sort

* fix: Add a sanitization function and do further checks

* fix:Switch to local variables

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Refactor authentication method check in auth.php

* Fix HTML encoding in goBack variable assignment

* Change get_order_string_page parameter to false

* Apply suggestion from @Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix error message capitalization for aggregate graph

* Apply suggestion from @Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Apply suggestion from @Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Apply suggestion from @Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix goBack link formatting in permission_denied.php

* Update lib/html_utility.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Add type hints to sanitize_uri and validation functions

* Fix comment formatting in validate_redirect_url

* Fix header location formatting in auth.php

* fix: Fixing PSR Issues with remote agent

* fix: Additional psr comments

* fix: More psr comments

* fix: PHPStan Issues

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
TheWitnessandCopilot authored and GitHub committed 2026-04-05 10:56:17 -04:00
1 parent a053260db8
commit 2810342f65
10 files changed
+203 -113

No files matched your search

+11 -12
View File
@@ -1019,13 +1019,15 @@ function graph_edit() : bool {
if (isrv('reset')) {
$_SESSION['aggregate_referer'] = 'aggregate_graphs.php';
} elseif (isset($_SERVER['HTTP_REFERER']) && !substr_count($_SERVER['HTTP_REFERER'], 'aggregate_graphs.php')) {
$_SESSION['aggregate_referer'] = $_SERVER['HTTP_REFERER'];
} elseif (isset($_SERVER['HTTP_REFERER']) && !isset($_SESSION['aggregate_referer'])) {
$_SESSION['aggregate_referer'] = $_SERVER['HTTP_REFERER'];
} elseif (isset($_SERVER['HTTP_REFERER'])) {
if (!isset($_SESSION['aggregate_referer'])) {
$_SESSION['aggregate_referer'] = validate_redirect_url($_SERVER['HTTP_REFERER'], 'aggregate_graphs.php');
}
} else {
$_SESSION['aggregate_referer'] = 'aggregate_graphs.php';
}
$referer = isset($_SESSION['aggregate_referer']) ? $_SESSION['aggregate_referer'] : 'aggregate_graphs.php';
$referer = $_SESSION['aggregate_referer'];
$use_graph_template = false;
$aginfo = [];
@@ -1074,14 +1076,11 @@ function graph_edit() : bool {
} elseif (cacti_sizeof($graphs)) {
$template = [];
} else {
raise_message('missing_aggregate', __('Aggregate Graphs Accessed does not Exist'), MESSAGE_LEVEL_ERROR);
raise_message('missing_aggregate', __('Aggregate Graph Accessed does not exist'), MESSAGE_LEVEL_ERROR);
if (isset($_SERVER['HTTP_REFERER'])) {
$referer = $_SERVER['HTTP_REFERER'];
header('Location: ' . $referer);
} else {
header('Location: aggregate_graphs.php');
}
$referer = validate_redirect_url($_SERVER['HTTP_REFERER'] ?? '', 'aggregate_graphs.php');
header('Location: ' . $referer);
exit;
}
+25 -36
View File
@@ -40,37 +40,34 @@ switch ($action) {
exit;
default:
// If the user is not logged in, redirect them to the login page
/**
* If the user is not logged in, redirect them back to the page
* they came from, or to the login page.
*/
if (!isset($_SESSION[SESS_USER_ID])) {
/* HTTP_REFERER redirect here is overridden by the unconditional
* index.php header below; use the safe fallback directly. */
header('Location: index.php');
$referer = validate_redirect_url($_SERVER['HTTP_REFERER'] ?? '', 'index.php');
header("Location: $referer");
exit;
}
}
if (isset($_SERVER['HTTP_REFERER'])) {
$return = $_SERVER['HTTP_REFERER'];
$return = validate_redirect_url($_SERVER['HTTP_REFERER'] ?? '', 'index.php');
if (basename($return) != 'auth_changepassword.php') {
if (str_contains($return, '/plugins/')) {
$parts = explode('/plugins/', $return);
$return = CACTI_PATH_URL . 'plugins/' . $parts[1];
} else {
$return = CACTI_PATH_URL . basename($return);
}
$_SESSION['acp_return'] = $return;
if (basename($return) != 'auth_changepassword.php') {
if (strpos($return, '/plugins/') !== false) {
$parts = explode('/plugins/', $return);
$return = CACTI_PATH_URL . 'plugins/' . $parts[1];
} else {
if (isset($_SESSION['acp_return'])) {
$return = $_SESSION['acp_return'];
} else {
$return = CACTI_PATH_URL . 'index.php';
}
$return = CACTI_PATH_URL . basename($return);
}
$_SESSION['acp_return'] = $return;
} else {
$return = CACTI_PATH_URL . 'index.php';
if (isset($_SESSION['acp_return'])) {
$return = $_SESSION['acp_return'];
} else {
$return = CACTI_PATH_URL . 'index.php';
}
}
$user = db_fetch_row_prepared('SELECT *
@@ -89,13 +86,9 @@ if (!cacti_sizeof($user) || $user['realm'] != 0) {
raise_message('nodomainpassword');
}
if (isset($_SERVER['HTTP_REFERER'])) {
// global.php already sanitized HTTP_REFERER; validate host before redirect.
$_ref = $_SERVER['HTTP_REFERER'];
header('Location: ' . ((parse_url($_ref, PHP_URL_HOST) === null || parse_url($_ref, PHP_URL_HOST) === $_SERVER['HTTP_HOST']) ? $_ref : 'index.php'));
} else {
header('Location: index.php');
}
$return = validate_redirect_url($_SERVER['HTTP_REFERER'] ?? '', 'index.php');
header("Location: $return");
exit;
}
@@ -107,13 +100,9 @@ if ($user['password_change'] != 'on') {
kill_session_var(SESS_USER_ID);
cacti_cookie_logout();
if (isset($_SERVER['HTTP_REFERER'])) {
// global.php already sanitized HTTP_REFERER; validate host before redirect.
$_ref = $_SERVER['HTTP_REFERER'];
header('Location: ' . ((parse_url($_ref, PHP_URL_HOST) === null || parse_url($_ref, PHP_URL_HOST) === $_SERVER['HTTP_HOST']) ? $_ref : 'index.php'));
} else {
header('Location: index.php');
}
$return = validate_redirect_url($_SERVER['HTTP_REFERER'] ?? '', 'index.php');
header("Location: $return");
exit;
}
+7 -11
View File
@@ -305,20 +305,16 @@ function settings() : bool {
return false;
}
if (isset($_SERVER['HTTP_REFERER'])) {
$referer = $_SERVER['HTTP_REFERER'];
$referer = validate_redirect_url($_SERVER['HTTP_REFERER'] ?? '', 'graph_view.php');
if (!str_contains($referer, 'auth_profile.php')) {
$timespan_sel_pos = strpos($referer, '&predefined_timespan');
if (strpos($referer, 'auth_profile.php') === false) {
$timespan_sel_pos = strpos($referer, '&predefined_timespan');
if ($timespan_sel_pos !== false) {
$referer = substr($referer, 0, $timespan_sel_pos);
}
$_SESSION['profile_referer'] = $referer;
if ($timespan_sel_pos !== false) {
$referer = substr($referer, 0, $timespan_sel_pos);
}
} elseif (!isset($_SESSION['profile_referer'])) {
$_SESSION['profile_referer'] = 'graph_view.php';
$_SESSION['profile_referer'] = $referer;
}
form_start('auth_profile.php', 'chk');
+4 -18
View File
@@ -1009,24 +1009,10 @@ function graphs() : void {
form_hidden_box('host_template_id', $host['host_template_id'], '0');
}
if (isset($_SERVER['HTTP_REFERER']) && $_SERVER['HTTP_REFERER'] != '') {
$referer_url = parse_url($_SERVER['HTTP_REFERER']);
if ($_SERVER['SERVER_NAME'] != $referer_url['host']) {
// Potential security exploit 1
srv('returnto', 'host.php');
} elseif (!str_contains($_SERVER['HTTP_REFERER'], 'graphs_new')) {
srv('returnto', basename($_SERVER['HTTP_REFERER']));
} else {
srv('returnto', 'host.php');
}
} elseif (isrv('returnto') && gnrv('returnto') != '') {
$returnto_url = parse_url(gnrv('returnto'));
if ($_SERVER['SERVER_NAME'] != $returnto_url['host']) {
// Potential security exploit 2
srv('returnto', 'host.php');
}
if (isset($_SERVER['HTTP_REFERER'])) {
set_request_var('returnto', validate_redirect_url($_SERVER['HTTP_REFERER'], 'host.php'));
} elseif (isset_request_var('returnto')) {
set_request_var('returnto', validate_redirect_url(get_nfilter_request_var('returnto'), 'host.php'));
}
load_current_session_value('returnto', 'sess_grn_returnto', '');
+2 -3
View File
@@ -74,8 +74,7 @@ api_plugin_hook_function('auth_alternate_realms');
*/
if ($auth_method != AUTH_METHOD_BASIC) {
if (isset($_SESSION[SESS_CHANGE_PASSWORD])) {
header('Location: ' . CACTI_PATH_URL . 'auth_changepassword.php?ref=' . ($_SERVER['HTTP_REFERER'] ?? 'index.php'));
header('Location: ' . CACTI_PATH_URL . 'auth_changepassword.php?ref=' . rawurlencode(validate_redirect_url($_SERVER['HTTP_REFERER'] ?? '', 'index.php')));
exit;
}
@@ -322,7 +321,7 @@ if (empty($_SESSION[SESS_USER_ID])) {
}
if (isset($_SERVER['HTTP_REFERER'])) {
$goBack = "<td colspan='2' class='center'>[<a href='" . $_SERVER['HTTP_REFERER'] . "'>" . __('Return') . "</a> | <a href='" . CACTI_PATH_URL . "logout.php'>" . __('Login Again') . '</a>]</td>';
$goBack = "<td colspan='2' class='center'>[<a href='" . validate_redirect_url($_SERVER['HTTP_REFERER'], $_SERVER['SCRIPT_NAME']) . "'>" . __('Return') . "</a> | <a href='" . CACTI_PATH_URL . "logout.php'>" . __('Login Again') . '</a>]</td>';
} elseif ($auth_method != AUTH_METHOD_BASIC) {
$goBack = "<td colspan='2' class='center'>[<a href='" . CACTI_PATH_URL . "logout.php'>" . __('Login Again') . '</a>]</td>';
} else {
+45 -5
View File
@@ -5046,9 +5046,34 @@ function sanitize_search_string(string $string) : string {
*
* @return string The sanitized uri
*/
function sanitize_uri($uri) {
static $drop_char_match = ['^', '$', '<', '>', '`', "'", '"', '|', '+', '[', ']', '{', '}', ';', '!', '(', ')'];
static $drop_char_replace = [ '', '', '', '', '', '', '', '', '', '', '', '', '', '', ''];
function sanitize_uri(string $uri) : string {
static $drop_char_match = [
'^', '$',
'<', '>',
'`', "'",
'"', '|',
'+', '[',
']', '{',
'}', ';',
'!', '(',
')'
];
static $drop_char_replace = [
'', '',
'', '',
'', '',
'', '',
'', '',
'', '',
'', '',
'', '',
''
];
if (is_urlencoded($uri)) {
$uri = urldecode($uri);
}
if (str_contains($uri, 'graph_view.php')) {
if (!strpos($uri, 'action=')) {
@@ -5056,7 +5081,22 @@ function sanitize_uri($uri) {
}
}
return str_replace($drop_char_match, $drop_char_replace, strip_tags(urldecode($uri)));
return str_replace($drop_char_match, $drop_char_replace, strip_tags($uri));
}
/**
* Checks to see if a string is urlencoded
*
* @param string $string the string to be validated
*
* @return boolean - true is the string is urlencoded otherwise false
*/
function is_urlencoded(string $string) : bool {
if ($string != urldecode($string)) {
return true;
} else {
return false;
}
}
/**
@@ -5066,7 +5106,7 @@ function sanitize_uri($uri) {
*
* @return bool true is the string is base64 otherwise false
*/
function is_base64_encoded($data) {
function is_base64_encoded(string $data) : bool {
// Perform a simple check first
if (!preg_match('/^[a-zA-Z0-9\/\r\n+]*={0,2}$/', $data)) {
return false;
+100 -8
View File
@@ -1250,7 +1250,7 @@ function validate_store_request_vars(array $filters, string $sess_prefix = '') :
* @return void
*/
function update_order_string(bool $inplace = false) : void {
$page = get_order_string_page();
$page = get_order_string_page(false);
$order = '';
@@ -1351,19 +1351,37 @@ function update_order_string(bool $inplace = false) : void {
* @return string The generated ORDER BY clause.
*/
function get_order_string() : string {
$page = get_order_string_page();
$page = get_order_string_page(true);
$sort_column = get_nfilter_request_var('sort_column');
$sort_dir = strtoupper(get_nfilter_request_var('sort_direction'));
if (!str_contains(get_request_var('sort_column'), '(') && !str_contains(get_request_var('sort_column'), '`')) {
if ($sort_dir !== 'ASC' && $sort_dir !== 'DESC') {
$sort_dir = 'ASC';
}
if (strpos($sort_column, '(') === false && strpos($sort_column, '`') === false) {
$del = '`';
} else {
$del = '';
}
/**
* Allowlist: identifiers are word chars and dots only; anything else could escape
* backtick quoting. Validate direction to prevent SQL keyword injection.
*/
if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_.()]*$/', $sort_column)) {
$sort_column = '';
}
if (isset($_SESSION['sort_string'][$page])) {
return $_SESSION['sort_string'][$page];
} else {
return 'ORDER BY ' . $del . implode($del . '.' . $del, explode('.', get_request_var('sort_column'))) . $del . ' ' . get_request_var('sort_direction');
}
if ($sort_column != '') {
return 'ORDER BY ' . $del . implode($del . '.' . $del, explode('.', $sort_column)) . $del . ' ' . $sort_dir;
}
return '';
}
/**
@@ -1378,7 +1396,7 @@ function get_order_string() : string {
* @return void
*/
function remove_column_from_order_string(string $column) : void {
$page = get_order_string_page();
$page = get_order_string_page(false);
if (isset($_SESSION['sort_data'][$page][$column])) {
unset($_SESSION['sort_data'][$page][$column]);
@@ -1394,9 +1412,11 @@ function remove_column_from_order_string(string $column) : void {
* such as 'action' and 'tab'. The page count is incremented with each call to
* ensure uniqueness.
*
* @param bool $increment Increment the page counter if true
*
* @return string A unique order string for the current page.
*/
function get_order_string_page() : string {
function get_order_string_page(bool $increment = true) : string {
static $page_count = 0;
$page = $page_count . '_' . str_replace('.php', '', get_current_page());
@@ -1409,11 +1429,83 @@ function get_order_string_page() : string {
$page .= '_' . get_nfilter_request_var('tab');
}
$page_count++;
if ($increment == true) {
$page_count++;
}
return $page;
}
/**
* Validate the redirect url provider by the HTTP_REFERER from PHP
*
* @param string $url The regular expression to validate.
* @param string $default The URL to travel to upon failure
*
* @return string The validated URL, or the provided $default if invalid
*/
function validate_redirect_url($url = '', $default = 'index.php') {
if ($url === '') {
return $default;
}
$url = trim($url);
// Decode the url to make it readable if encoded
if (is_urlencoded($url)) {
$url = urldecode($url);
}
// reject URLs with protocol schemes (external redirects, javascript:, data:)
$bad_strings = [
'javascript:',
'data:',
'vbscript:',
'mailto:',
'file:'
];
foreach ($bad_strings as $bstring) {
if (stripos($url, $bstring) !== false) {
return $default;
}
}
// reject protocol-relative URLs
if (strpos($url, '//') === 0) {
return $default;
}
// reject URLs with newlines (header injection)
if (preg_match('/[\r\n]/', $url)) {
return $default;
}
// reject path traversal sequences
if (stripos($url, '..') !== false) {
return $default;
}
// Prevent referring off site
$ref_host = parse_url($url, PHP_URL_HOST);
$srv_host = null;
if (isset($_SERVER['HTTP_HOST']) && $_SERVER['HTTP_HOST'] != '') {
$srv_host = preg_replace('/:\d+$/', '', $_SERVER['HTTP_HOST']);
}
if ($ref_host === null || ($srv_host !== null && $ref_host === $srv_host)) {
$ref_path = parse_url($url, PHP_URL_PATH) ?: '';
$ref_query = parse_url($url, PHP_URL_QUERY);
$safe = sanitize_uri($ref_path . ($ref_query !== null ? '?' . $ref_query : ''));
return $safe;
} else {
return $default;
}
}
/**
* Validates if the given string is a valid regular expression.
*
+4 -18
View File
@@ -32,24 +32,10 @@ $page = db_fetch_row_prepared('SELECT
// Prevent redirect loops
if (isset($_SERVER['HTTP_REFERER'])) {
if (!str_contains($_SERVER['HTTP_REFERER'], 'link.php')) {
/* Reject non-http(s) schemes (javascript:, data:, etc.) before any
* host comparison; parse_url returns null for host on those schemes,
* which would otherwise pass the === null branch unchanged. Compare
* against SERVER_NAME (web-server config) rather than HTTP_HOST
* (attacker-supplied request header) to prevent host-spoofing.
* parse_url() returns false on a completely malformed URL; false fails
* all === comparisons below, so a malformed referer falls back to
* 'index.php'. null means the component is absent (e.g. relative path),
* which is allowed through as a same-host local referer. */
$raw = $_SERVER['HTTP_REFERER'];
$scheme = parse_url($raw, PHP_URL_SCHEME);
$host = parse_url($raw, PHP_URL_HOST);
$referer = (
($scheme === null || $scheme === 'http' || $scheme === 'https') &&
($host === null || $host === ($_SERVER['SERVER_NAME'] ?? ''))
) ? sanitize_uri($raw) : 'index.php';
$_SESSION['link_referer'] = $referer;
if (strpos($_SERVER['HTTP_REFERER'], 'link.php') === false) {
$_SESSION['link_referer'] = validate_redirect_url($_SERVER['HTTP_REFERER'], 'index.php');
$referer = $_SESSION['link_referer'];
} elseif (isset($_SESSION['link_referer'])) {
$referer = sanitize_uri($_SESSION['link_referer']);
} else {
+3 -2
View File
@@ -27,9 +27,10 @@ require('./include/auth.php');
$version = CACTI_VERSION_TEXT;
if (isset($_SERVER['HTTP_REFERER'])) {
$goBack = "[<a href='" . sanitize_uri($_SERVER['HTTP_REFERER']) . "'>" . __('Return') . "</a> | <a href='" . CACTI_PATH_URL . "logout.php'>" . __('Login Again') . '</a>]';
$redirect = validate_redirect_url($_SERVER['HTTP_REFERER'], 'index.php');
$goBack = "[<a href='" . $redirect . "'>" . __('Return') . "</a> | <a href='" . CACTI_PATH_URL . "logout.php'>" . __('Login Again') . '</a>]';
} else {
$goBack = "[<a href='#' onClick='window.history.back()'>" . __('Return') . "</a> | <a href='" . CACTI_PATH_URL . "logout.php'>" . __('Login Again') . '</a>]';
$goBack = "[<a href='#' onClick='window.history.back()'>" . __('Return') . "</a> | <a href='" . CACTI_PATH_URL . "logout.php'>" . __('Login Again') . '</a>]';
}
// allow for plugin based permission denied page
+2
View File
@@ -284,6 +284,7 @@ function get_snmp_data() : void {
if (!is_string($oid) || !preg_match('/^[0-9.]+$/', $oid)) {
print 'U';
return;
}
@@ -313,6 +314,7 @@ function get_snmp_data_walk() : void {
if (!is_string($oid) || !preg_match('/^[0-9.]+$/', $oid)) {
print 'U';
return;
}