mirror of
https://github.com/Cacti/cacti.git
synced 2026-10-23 03:27:02 +09:00
security: batch 8 (develop) - data-query XML oracle (GHSA-2x86) + batch-8 GHSAs under 1.2.32 CHANGELOG (#8151)
* security: batch 8 develop - confine data-query XML existence check (GHSA-2x86); list batch-8 GHSAs under the 1.2.32 CHANGELOG release * security: batch 8 (develop) - address review: use cacti_path_is_within() for XML existence-oracle containment with a NUL-byte guard (no realpath ValueError); add data_query_edit() UI hardening regression test --------- Co-authored-by: Larry Adams <TheWitness@users.noreply.github.com>
This commit is contained in:
1 parent
9facd9b500
commit
cff89ca9d6
3 files changed
+257
-3
No files matched your search
@@ -2,8 +2,6 @@ Cacti CHANGELOG
|
||||
|
||||
1.3.0-dev
|
||||
-security#GHSA-4p7f-qcc2-vmx7: Escape path_php_binary before the resource-cache PHP syntax-check shell command
|
||||
-security#GHSA-m6wx-f538-m6q3: Apply the .log extension whitelist to path_boost_log to block log-poisoning RCE
|
||||
-security#GHSA-73qf-hq23-c4m7: Authorize tree lock, unlock, and get_node against per-tree access before dispatch
|
||||
-security#GHSA-967c-6qj7-q7rh: Confirm the rrdtool control-character strip closes the RRD-create |query_*| injection path
|
||||
-security#GHSA-f7jw-gfhr-cxwm: Escape the automation tree-rule preview Resulting Branch values
|
||||
-security#GHSA-rp5g-r5vp-q7j6: Quote the db_column_exists() LIKE term so the SQLi-defense helper is not itself injectable
|
||||
@@ -415,6 +413,15 @@ Cacti CHANGELOG
|
||||
-feature#7681: Add fail2ban support with a stable authentication-failure log line and a bundled filter
|
||||
|
||||
1.2.32
|
||||
-security#GHSA-227p-wv52-pjg5: Escape the spike-removal RRD path passed to the removespikes shell command
|
||||
-security#GHSA-2x86-jpm8-9vgp: Confine the data-query XML file existence check to the Cacti base path
|
||||
-security#GHSA-4gph-8x7q-h365: Escape the device description in the report-add status message
|
||||
-security#GHSA-73qf-hq23-c4m7: Authorize the tree copy, lock and unlock actions against per-tree access before mutating
|
||||
-security#GHSA-f7jw-gfhr-cxwm: Escape the Resulting Branch value in the automation tree-rule matching preview
|
||||
-security#GHSA-h5wg-qf6f-7f6r: Reject unsafe identifiers in the plugin table create and drop helpers
|
||||
-security#GHSA-m57p-vf3v-cpgg: Escape the SNMP-sourced device description in the automation tree-rule matching preview
|
||||
-security#GHSA-m6wx-f538-m6q3: Enforce the .log extension on the Boost debug log path setting
|
||||
-security#GHSA-r6jp-ff29-v9qw: Escape the imported template name in the template-import preview
|
||||
-security#GHSA-4rmr-wvjq-qxc2: Run the per-page realm check after Basic Auth and guest login instead of returning early
|
||||
-security#GHSA-j67j-wpm4-9g3x: Require a CSRF-token POST for the user and group update_policy action
|
||||
-security#GHSA-2p3x-f5jm-rg6p: Break the View-Graphs to poller remote code execution chain
|
||||
|
||||
+8
-1
@@ -1132,7 +1132,14 @@ function data_query_edit() : void {
|
||||
$replace = [CACTI_PATH_BASE, read_config_option('path_snmpget'), read_config_option('path_php_binary')];
|
||||
$xml_filename = str_replace($search, $replace, $snmp_query['xml_path']);
|
||||
|
||||
if ((file_exists($xml_filename)) && (is_file($xml_filename))) {
|
||||
// Confine the resolved path to the Cacti base path so this located/not-located
|
||||
// check cannot be used as a filesystem existence oracle (GHSA-2x86-jpm8-9vgp).
|
||||
// cacti_path_is_within() normalizes Windows casing/long-path forms and matches
|
||||
// the containment check used by the XML loader in lib/data_query.php. Reject a
|
||||
// NUL byte first so realpath() cannot throw a ValueError on PHP 8+.
|
||||
if ($xml_filename !== '' && strpos($xml_filename, "\0") === false &&
|
||||
cacti_path_is_within($xml_filename, CACTI_PATH_BASE) &&
|
||||
is_file($xml_filename)) {
|
||||
$text = "<span class='deviceUp'>" . __('Successfully located XML file') . '</span>';
|
||||
$xml_file_exists = true;
|
||||
} else {
|
||||
|
||||
@@ -0,0 +1,240 @@
|
||||
<?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 |
|
||||
+-------------------------------------------------------------------------+
|
||||
*/
|
||||
|
||||
/**
|
||||
* Tests for the data query XML path existence-oracle hardening in the
|
||||
* data_query_edit() UI page (data_queries.php).
|
||||
*
|
||||
* This is the separate UI code path from get_data_query_array() (covered by
|
||||
* DataQueryXmlPathHardeningTest.php). When rendering the edit page, Cacti
|
||||
* reports whether the configured XML file was "located"; without containment,
|
||||
* that located/not-located banner leaks the existence of arbitrary files
|
||||
* (GHSA-2x86-jpm8-9vgp). The guard confines the resolved path to the Cacti
|
||||
* base via cacti_path_is_within() and rejects a NUL byte before the filesystem
|
||||
* call so realpath() cannot throw a ValueError on PHP 8+.
|
||||
*
|
||||
* Two test suites:
|
||||
* 1. Source-scan: verifies the guard structure is textually present.
|
||||
* 2. Runtime boundary: exercises the real cacti_path_is_within() guard in
|
||||
* isolation against a temporary directory tree, including the NUL-byte
|
||||
* boundary that runs during page rendering.
|
||||
*/
|
||||
|
||||
require_once dirname(__DIR__, 4) . '/lib/functions.php';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Source-scan suite (secondary lint — does NOT substitute for runtime tests)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
$src = file_get_contents(dirname(__DIR__, 4) . '/data_queries.php');
|
||||
|
||||
test('data_query_edit confines the XML existence check with cacti_path_is_within', function () use ($src) {
|
||||
expect($src)->toContain('cacti_path_is_within($xml_filename, CACTI_PATH_BASE)');
|
||||
});
|
||||
|
||||
test('data_query_edit rejects a NUL byte before the filesystem call', function () use ($src) {
|
||||
expect($src)->toContain('strpos($xml_filename, "\0") === false');
|
||||
});
|
||||
|
||||
test('data_query_edit reads existence via is_file on the confined filename', function () use ($src) {
|
||||
expect($src)->toContain('is_file($xml_filename)');
|
||||
});
|
||||
|
||||
test('data_query_edit no longer uses a raw case-sensitive prefix comparison', function () use ($src) {
|
||||
expect($src)->not->toContain('strpos($xml_real, $base_real . DIRECTORY_SEPARATOR) === 0');
|
||||
});
|
||||
|
||||
test('NUL-byte guard precedes the cacti_path_is_within() call', function () use ($src) {
|
||||
$nul_pos = strpos($src, 'strpos($xml_filename, "\0") === false');
|
||||
$within_pos = strpos($src, 'cacti_path_is_within($xml_filename, CACTI_PATH_BASE)');
|
||||
|
||||
expect($nul_pos)->not->toBeFalse()
|
||||
->and($within_pos)->not->toBeFalse()
|
||||
->and($nul_pos)->toBeLessThan($within_pos);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Runtime boundary suite — exercises the real guard logic in isolation
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/*
|
||||
* Replicates the guard from data_query_edit() so it can be exercised without
|
||||
* the full Cacti bootstrap (DB, session, page rendering). It calls the real
|
||||
* cacti_path_is_within() from lib/functions.php, so traversal/symlink/casing
|
||||
* behaviour is covered by the production helper, not a re-implementation.
|
||||
*
|
||||
* Returns true only when $xml_filename is non-empty, contains no NUL byte,
|
||||
* resolves within $base, and is a regular file.
|
||||
*/
|
||||
function dataQueryEditGuardAllowed(string $base, string $xml_filename): bool {
|
||||
if ($xml_filename === '' || strpos($xml_filename, "\0") !== false) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return cacti_path_is_within($xml_filename, $base) && is_file($xml_filename);
|
||||
}
|
||||
|
||||
function makeDQEditTempBase(): string {
|
||||
$tmp = sys_get_temp_dir() . '/cacti_dq_edit_test_' . getmypid() . '_' . bin2hex(random_bytes(4));
|
||||
mkdir($tmp . '/resource/snmp-queries', 0755, true);
|
||||
mkdir($tmp . '/outside', 0755, true);
|
||||
|
||||
return $tmp;
|
||||
}
|
||||
|
||||
function removeDQEditTempBase(string $base): void {
|
||||
foreach (['resource/snmp-queries', 'resource', 'outside'] as $sub) {
|
||||
$path = $base . '/' . $sub;
|
||||
|
||||
if (is_file($path)) {
|
||||
unlink($path);
|
||||
} elseif (is_dir($path)) {
|
||||
rmdir($path);
|
||||
}
|
||||
}
|
||||
|
||||
if (is_dir($base)) {
|
||||
rmdir($base);
|
||||
}
|
||||
}
|
||||
|
||||
// --- happy path ---
|
||||
|
||||
test('xml file inside the base is located', function () {
|
||||
$base = makeDQEditTempBase();
|
||||
$file = $base . '/resource/snmp-queries/interface.xml';
|
||||
file_put_contents($file, '<query/>');
|
||||
|
||||
$result = dataQueryEditGuardAllowed($base, $file);
|
||||
|
||||
unlink($file);
|
||||
removeDQEditTempBase($base);
|
||||
|
||||
expect($result)->toBeTrue();
|
||||
});
|
||||
|
||||
// --- traversal / out-of-base ---
|
||||
|
||||
test('traversal escaping the base is not located', function () {
|
||||
$tmp = sys_get_temp_dir() . '/cacti_dq_edit_trav_' . getmypid() . '_' . bin2hex(random_bytes(4));
|
||||
$base = $tmp . '/cacti';
|
||||
mkdir($base . '/resource/snmp-queries', 0755, true);
|
||||
mkdir($tmp . '/outside', 0755, true);
|
||||
$evil = $tmp . '/outside/passwd';
|
||||
file_put_contents($evil, 'root:x:0:0');
|
||||
|
||||
$traversal = $base . '/resource/snmp-queries/../../../outside/passwd';
|
||||
$result = dataQueryEditGuardAllowed($base, $traversal);
|
||||
|
||||
unlink($evil);
|
||||
rmdir($tmp . '/outside');
|
||||
rmdir($base . '/resource/snmp-queries');
|
||||
rmdir($base . '/resource');
|
||||
rmdir($base);
|
||||
rmdir($tmp);
|
||||
|
||||
expect($result)->toBeFalse();
|
||||
});
|
||||
|
||||
test('an existing absolute path outside the base is not located', function () {
|
||||
$base = makeDQEditTempBase();
|
||||
$result = dataQueryEditGuardAllowed($base, sys_get_temp_dir());
|
||||
|
||||
removeDQEditTempBase($base);
|
||||
|
||||
expect($result)->toBeFalse();
|
||||
});
|
||||
|
||||
test('a non-existent in-base path is not located', function () {
|
||||
$base = makeDQEditTempBase();
|
||||
$result = dataQueryEditGuardAllowed($base, $base . '/resource/snmp-queries/missing.xml');
|
||||
|
||||
removeDQEditTempBase($base);
|
||||
|
||||
expect($result)->toBeFalse();
|
||||
});
|
||||
|
||||
// --- NUL-byte boundary (runs during page rendering, must not throw) ---
|
||||
|
||||
test('a NUL byte in the filename is rejected without raising a ValueError', function () {
|
||||
$base = makeDQEditTempBase();
|
||||
$file = $base . "/resource/snmp-queries/interface.xml\0.evil";
|
||||
|
||||
// On PHP 8+, realpath() throws a ValueError on a NUL byte; the guard must
|
||||
// short-circuit before reaching cacti_path_is_within()/realpath().
|
||||
$result = dataQueryEditGuardAllowed($base, $file);
|
||||
|
||||
removeDQEditTempBase($base);
|
||||
|
||||
expect($result)->toBeFalse();
|
||||
});
|
||||
|
||||
test('an empty filename is not located', function () {
|
||||
$base = makeDQEditTempBase();
|
||||
$result = dataQueryEditGuardAllowed($base, '');
|
||||
|
||||
removeDQEditTempBase($base);
|
||||
|
||||
expect($result)->toBeFalse();
|
||||
});
|
||||
|
||||
// --- symlink pivots ---
|
||||
|
||||
test('a symlink inside the base resolving to a file inside the base is located', function () {
|
||||
$base = makeDQEditTempBase();
|
||||
$target = $base . '/resource/snmp-queries/interface.xml';
|
||||
file_put_contents($target, '<query/>');
|
||||
|
||||
$link = $base . '/resource/snmp-queries/iface-link.xml';
|
||||
symlink($target, $link);
|
||||
|
||||
$result = dataQueryEditGuardAllowed($base, $link);
|
||||
|
||||
unlink($link);
|
||||
unlink($target);
|
||||
removeDQEditTempBase($base);
|
||||
|
||||
expect($result)->toBeTrue();
|
||||
});
|
||||
|
||||
test('a symlink inside the base resolving outside the base is not located', function () {
|
||||
$base = makeDQEditTempBase();
|
||||
$external = sys_get_temp_dir() . '/cacti_dq_edit_ext_' . getmypid() . '_' . bin2hex(random_bytes(4));
|
||||
mkdir($external, 0755, true);
|
||||
$ext_file = $external . '/secret.txt';
|
||||
file_put_contents($ext_file, 'secret');
|
||||
|
||||
$link = $base . '/resource/snmp-queries/evil-link.xml';
|
||||
symlink($ext_file, $link);
|
||||
|
||||
$result = dataQueryEditGuardAllowed($base, $link);
|
||||
|
||||
unlink($link);
|
||||
unlink($ext_file);
|
||||
rmdir($external);
|
||||
removeDQEditTempBase($base);
|
||||
|
||||
expect($result)->toBeFalse();
|
||||
});
|
||||
|
||||
// --- directory inside base: is_file() rejects it even though it is contained ---
|
||||
|
||||
test('a directory inside the base is not located because is_file rejects it', function () {
|
||||
$base = makeDQEditTempBase();
|
||||
$result = dataQueryEditGuardAllowed($base, $base . '/resource/snmp-queries');
|
||||
|
||||
removeDQEditTempBase($base);
|
||||
|
||||
expect($result)->toBeFalse();
|
||||
});
|
||||
Reference in new issue
Block a user