Skip to content

Commit ecf2c16

Browse files
security: fix XSS and open redirect in auth and UI pages (1.2.x backport)
- Validate Referer host before using in Location header (auth_changepassword.php, link.php) - Encode return target via json_encode in JS onClick handler (auth_changepassword.php) - Encode tab parameter via json_encode in JS context (auth_profile.php) - Encode pageAction and graphPage via json_encode in JS context (lib/html_graph.php) - Add JS context hardening unit tests Addresses GHSA-34rf-frc3-v48r (High), GHSA-2j98-xfjq-gw39 (Medium), GHSA-cfhh-pwvx-gp5g (Medium), GHSA-rv79-cxhv-2jwq (Medium) Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
1 parent cea6212 commit ecf2c16

File tree

5 files changed

+99
-7
lines changed

5 files changed

+99
-7
lines changed

auth_changepassword.php

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,10 @@
9292
}
9393

9494
if (isset($_SERVER['HTTP_REFERER'])) {
95-
header('Location: ' . $_SERVER['HTTP_REFERER']);
95+
$_ref = sanitize_uri($_SERVER['HTTP_REFERER']);
96+
$_ref_host = parse_url($_ref, PHP_URL_HOST);
97+
$_srv_host = preg_replace('/:\d+$/', '', $_SERVER['HTTP_HOST']);
98+
header('Location: ' . (($_ref_host === null || $_ref_host === $_srv_host) ? $_ref : 'index.php'));
9699
} else {
97100
header('Location: index.php');
98101
}
@@ -108,7 +111,10 @@
108111
cacti_cookie_logout();
109112

110113
if (isset($_SERVER['HTTP_REFERER'])) {
111-
header('Location: ' . $_SERVER['HTTP_REFERER']);
114+
$_ref = sanitize_uri($_SERVER['HTTP_REFERER']);
115+
$_ref_host = parse_url($_ref, PHP_URL_HOST);
116+
$_srv_host = preg_replace('/:\d+$/', '', $_SERVER['HTTP_HOST']);
117+
header('Location: ' . (($_ref_host === null || $_ref_host === $_srv_host) ? $_ref : 'index.php'));
112118
} else {
113119
header('Location: index.php');
114120
}
@@ -387,7 +393,9 @@
387393
</tr>
388394
<tr>
389395
<td class='nowrap' colspan='2'><input type='submit' class='ui-button ui-corner-all ui-widget' value='<?php print __esc('Save'); ?>'>
390-
<?php print $user['must_change_password'] != 'on' ? "<input type='button' class='ui-button ui-corner-all ui-widget' onClick='document.location=\"$return\"' value='". __esc('Return') . "'>":"";?>
396+
<?php if ($user['must_change_password'] != 'on') { ?>
397+
<input type='button' class='ui-button ui-corner-all ui-widget' onClick='document.location=<?php print json_encode((string) $return); ?>' value='<?php print __esc('Return'); ?>'>
398+
<?php } ?>
391399
</td>
392400
</tr>
393401
</table>

auth_profile.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ function settings_javascript() {
456456
<script type='text/javascript'>
457457

458458
var themeFonts = <?php print read_config_option('font_method');?>;
459-
var currentTab = '<?php print get_nfilter_request_var('tab');?>';
459+
var currentTab = <?php print json_encode((string) get_nfilter_request_var('tab'));?>;
460460
var currentTheme = '<?php print get_selected_theme();?>';
461461
var currentLang = '<?php print read_config_option('user_language');?>';
462462
var authMethod = '<?php print read_config_option('auth_method');?>';

lib/html_graph.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,8 +353,8 @@ function html_graph_preview_filter($page, $action, $devices_where = '', $templat
353353
var graph_start = <?php print get_current_graph_start();?>;
354354
var graph_end = <?php print get_current_graph_end();?>;
355355
var timeOffset = <?php print date('Z');?>;
356-
var pageAction = '<?php print $action;?>';
357-
var graphPage = '<?php print $page;?>';
356+
var pageAction = <?php print json_encode($action);?>;
357+
var graphPage = <?php print json_encode($page);?>;
358358
var date1Open = false;
359359
var date2Open = false;
360360

link.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,10 @@
3333
// Prevent redirect loops
3434
if (isset($_SERVER['HTTP_REFERER'])) {
3535
if (strpos($_SERVER['HTTP_REFERER'], 'link.php') === false) {
36-
$referer = $_SERVER['HTTP_REFERER'];
36+
$raw = sanitize_uri($_SERVER['HTTP_REFERER']);
37+
$ref_host = parse_url($raw, PHP_URL_HOST);
38+
$srv_host = preg_replace('/:\d+$/', '', $_SERVER['HTTP_HOST']);
39+
$referer = ($ref_host === null || $ref_host === $srv_host) ? $raw : 'index.php';
3740
$_SESSION['link_referer'] = $referer;
3841
} elseif (isset($_SESSION['link_referer'])) {
3942
$referer = sanitize_uri($_SESSION['link_referer']);
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
| |
6+
| This program is free software; you can redistribute it and/or |
7+
| modify it under the terms of the GNU General Public License |
8+
| as published by the Free Software Foundation; either version 2 |
9+
| of the License, or (at your option) any later version. |
10+
+-------------------------------------------------------------------------+
11+
| Cacti: The Complete RRDtool-based Graphing Solution |
12+
+-------------------------------------------------------------------------+
13+
*/
14+
15+
/*
16+
* Regression checks for JavaScript-context output hardening.
17+
*
18+
* These tests ensure request/session derived values are encoded or normalized
19+
* before being embedded in JavaScript.
20+
*/
21+
22+
$authProfilePath = __DIR__ . '/../../auth_profile.php';
23+
$authResetpasswordPath = __DIR__ . '/../../auth_resetpassword.php';
24+
$authChangepasswordPath = __DIR__ . '/../../auth_changepassword.php';
25+
$pluginsPath = __DIR__ . '/../../plugins.php';
26+
$htmlGraphPath = __DIR__ . '/../../lib/html_graph.php';
27+
$dataDebugPath = __DIR__ . '/../../data_debug.php';
28+
29+
test('auth_profile encodes tab for JavaScript and redirect URL', function () use ($authProfilePath) {
30+
$contents = file_get_contents($authProfilePath);
31+
32+
expect($contents)->toContain("json_encode((string) grv('tab'))");
33+
expect($contents)->toContain("gfrv('tab', FILTER_VALIDATE_REGEXP");
34+
expect($contents)->toContain('rawurlencode($currentTab)');
35+
});
36+
37+
test('plugins page normalizes state and encodes sort column in JavaScript', function () use ($pluginsPath) {
38+
$contents = file_get_contents($pluginsPath);
39+
40+
expect($contents)->toContain("json_encode((string) grv('sort_column'))");
41+
expect($contents)->toContain("var tableState = <?php print (int) grv('state'); ?>;");
42+
expect($contents)->not->toContain("var tableState = <?php print grv('state'); ?>");
43+
});
44+
45+
test('graph list view uses JSON and sanitized CSV for graph list', function () use ($htmlGraphPath) {
46+
$contents = file_get_contents($htmlGraphPath);
47+
48+
expect($contents)->toContain('$graph_list_js = []');
49+
expect($contents)->toContain('ctype_digit($item)');
50+
expect($contents)->toContain('json_encode($graph_list_js)');
51+
expect($contents)->toContain("graph_list=<?php print \$graph_list_csv; ?>");
52+
expect($contents)->not->toContain("new Array(<?php print grv('graph_list'); ?>)");
53+
});
54+
55+
test('graph pages encode JS-bound PHP strings safely', function () use ($htmlGraphPath) {
56+
$contents = file_get_contents($htmlGraphPath);
57+
58+
expect($contents)->toContain('var pageAction = <?php print json_encode($action); ?>');
59+
expect($contents)->toContain('var graphPage = <?php print json_encode($page); ?>');
60+
expect($contents)->toContain("json_encode((string) \$suffix)");
61+
});
62+
63+
test('data debug escapes tooltip title values before rendering', function () use ($dataDebugPath) {
64+
$contents = file_get_contents($dataDebugPath);
65+
66+
expect($contents)->toContain('$value_title = htmle((string) $value);');
67+
});
68+
69+
test('auth reset password encodes return location in onclick handlers', function () use ($authResetpasswordPath) {
70+
$contents = file_get_contents($authResetpasswordPath);
71+
72+
expect($contents)->toContain("document.location=<?php print json_encode((string) \$return); ?>");
73+
expect($contents)->not->toContain("document.location=\"<?php print \$return; ?>\"");
74+
});
75+
76+
test('auth change password encodes return location in onclick handler', function () use ($authChangepasswordPath) {
77+
$contents = file_get_contents($authChangepasswordPath);
78+
79+
expect($contents)->toContain("document.location=<?php print json_encode((string) \$return); ?>");
80+
expect($contents)->not->toContain("onClick='document.location=\\\"\$return\\\"'");
81+
});

0 commit comments

Comments
 (0)