mirror of
https://github.com/Cacti/cacti.git
synced 2026-10-23 03:27:02 +09:00
Confine index.php external-link content includes (#7687)
* Confine index.php external-link content includes render_external_links() built the include path as include/content/<contentfile> and required it after only a file_exists() check. contentfile comes from the external_links table, so a '../' value included an arbitrary local file. Resolve and confine the path to include/content the way link.php already does, and html_escape the contentfile in the iframe src and error message. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Escape only the filename in the not-found message and guard the test offsets Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net>
This commit is contained in:
1 parent
775be6e479
commit
5ea5dbd693
3 files changed
+93
-4
No files matched your search
@@ -90,6 +90,7 @@ Cacti CHANGELOG
|
||||
-issue#7683: Harden login brute-force defenses with an atomic lockout counter and constant-time unknown-user handling
|
||||
-issue#7684: Scope the local-login password rehash to realm 0 and only rehash after a successful login
|
||||
-issue#7685: Validate the post-login referer redirect to close an open-redirect gap
|
||||
-issue#7686: Confine index.php external-link content includes to include/content and escape the filename
|
||||
-issue#7688: Force the SNMP port to an integer before it reaches the net-snmp command line
|
||||
-issue#7732: Bind device IDs and escape graph and device HTML sinks
|
||||
-feature#412: Import Export for Graph and Tree rules
|
||||
|
||||
@@ -39,16 +39,20 @@ function render_external_links(string $style = 'FRONT') : void {
|
||||
foreach ($consoles as $page) {
|
||||
if (is_realm_allowed($page['id'] + 10000)) {
|
||||
if (preg_match('/^((((ht|f)tp(s?))\:\/\/){1}\S+)/i', $page['contentfile'])) {
|
||||
print '<iframe class="content" src="' . $page['contentfile'] . '" frameborder="0"></iframe>';
|
||||
print '<iframe class="content" src="' . html_escape($page['contentfile']) . '" frameborder="0"></iframe>';
|
||||
} else {
|
||||
print '<div id="content">';
|
||||
|
||||
$file = CACTI_PATH_INCLUDE . '/content/' . $page['contentfile'];
|
||||
// Confine the include to include/content, the same way link.php
|
||||
// does. contentfile comes from the external_links table; without
|
||||
// this a '../' value would include an arbitrary local file.
|
||||
$basepath = realpath(CACTI_PATH_INCLUDE . '/content');
|
||||
$file = ($basepath !== false) ? realpath($basepath . '/' . $page['contentfile']) : false;
|
||||
|
||||
if (file_exists($file)) {
|
||||
if ($file !== false && is_file($file) && str_starts_with($file, $basepath . DIRECTORY_SEPARATOR)) {
|
||||
require_once($file);
|
||||
} else {
|
||||
print '<h1>The file \'' . $page['contentfile'] . '\' does not exist!!</h1>';
|
||||
print '<h1>The file \'' . html_escape($page['contentfile']) . '\' does not exist!!</h1>';
|
||||
}
|
||||
|
||||
print '</div>';
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
<?php
|
||||
/*
|
||||
+-------------------------------------------------------------------------+
|
||||
| Copyright (C) 2004-2026 The Cacti Group |
|
||||
| |
|
||||
| This program is free software; you can redistribute it and/or |
|
||||
| modify it under the terms of the GNU General Public License |
|
||||
| as published by the Free Software Foundation; either version 2 |
|
||||
| of the License, or (at your option) any later version. |
|
||||
+-------------------------------------------------------------------------+
|
||||
| Cacti: The Complete RRDtool-based Graphing Solution |
|
||||
+-------------------------------------------------------------------------+
|
||||
*/
|
||||
|
||||
/*
|
||||
* render_external_links() included external_links.contentfile from
|
||||
* include/content with only a file_exists() check, so a '../' value included an
|
||||
* arbitrary local file (stored LFI for a user with external-links write). It now
|
||||
* confines the resolved path to include/content the way link.php does, and
|
||||
* escapes the contentfile in the iframe src and the error message.
|
||||
*/
|
||||
|
||||
$src = file_get_contents(dirname(__DIR__, 2) . '/index.php');
|
||||
|
||||
function _render_links_body(string $src): string {
|
||||
expect($src)->not->toBeEmpty();
|
||||
$start = strpos($src, 'function render_external_links(');
|
||||
expect($start)->not->toBeFalse();
|
||||
$end = strpos($src, "\n}", $start);
|
||||
expect($end)->not->toBeFalse();
|
||||
|
||||
return substr($src, $start, $end - $start);
|
||||
}
|
||||
|
||||
test('the content include is confined to include/content before it is required', function () use ($src) {
|
||||
$body = _render_links_body($src);
|
||||
|
||||
$guard = strpos($body, 'str_starts_with($file, $basepath . DIRECTORY_SEPARATOR)');
|
||||
$require = strpos($body, 'require_once($file)');
|
||||
|
||||
expect($body)->toContain("\$basepath = realpath(CACTI_PATH_INCLUDE . '/content')");
|
||||
expect($body)->toContain('$file = ($basepath !== false) ? realpath($basepath');
|
||||
expect($guard)->not->toBeFalse();
|
||||
expect($require)->not->toBeFalse();
|
||||
// the confinement check gates the require
|
||||
expect($guard)->toBeLessThan($require);
|
||||
// the raw file_exists()->require path is gone
|
||||
expect($body)->not->toContain('if (file_exists($file)) {');
|
||||
});
|
||||
|
||||
test('a traversal contentfile does not resolve inside the content base', function () {
|
||||
// mirror the guard: realpath of a traversal escapes the base, so str_starts_with fails
|
||||
$base = sys_get_temp_dir() . '/cacti_content_' . getmypid();
|
||||
@mkdir($base, 0700, true);
|
||||
@mkdir($base . '/legit', 0700, true);
|
||||
file_put_contents($base . '/legit/ok.php', '<?php');
|
||||
file_put_contents(dirname($base) . '/evil.php', '<?php');
|
||||
|
||||
$within = static function (string $contentfile) use ($base): bool {
|
||||
$basepath = realpath($base);
|
||||
$file = ($basepath !== false) ? realpath($basepath . '/' . $contentfile) : false;
|
||||
|
||||
return $file !== false && is_file($file) && str_starts_with($file, $basepath . DIRECTORY_SEPARATOR);
|
||||
};
|
||||
|
||||
expect($within('legit/ok.php'))->toBeTrue();
|
||||
expect($within('../evil.php'))->toBeFalse();
|
||||
expect($within('legit/../../evil.php'))->toBeFalse();
|
||||
|
||||
@unlink($base . '/legit/ok.php');
|
||||
@unlink(dirname($base) . '/evil.php');
|
||||
@rmdir($base . '/legit');
|
||||
@rmdir($base);
|
||||
});
|
||||
|
||||
test('the contentfile is escaped in the iframe src and error output', function () use ($src) {
|
||||
$body = _render_links_body($src);
|
||||
|
||||
expect($body)->toContain("src=\"' . html_escape(\$page['contentfile'])");
|
||||
expect($body)->not->toContain("src=\"' . \$page['contentfile']");
|
||||
// the not-found error output escapes only the filename, matching link.php
|
||||
expect($body)->toContain("html_escape(\$page['contentfile']) . '\\' does not exist");
|
||||
expect($body)->not->toContain("html_escape('The file");
|
||||
});
|
||||
Reference in new issue
Block a user