mirror of
https://github.com/Cacti/cacti.git
synced 2026-10-23 19:47:06 +09:00
* fix(rrdtool): quote RRDfile paths passed to the command line __rrd_execute() hands the assembled line to shell_exec(), and its docblock already required callers to quote each variable argument. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(tests): widen bare RRD path sweep Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(rrdtool): quote binary path as one argument Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(rrd): preserve caller logging context Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Make RRD XML mutations transactional Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Harden RRDtool proxy transport handling Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(rrdproxy): reject invalid command argument lists Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(rrdproxy): preserve binary graph payloads Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Keep RRDtool proxy sessions encrypted Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Harden local RRDtool command and output handoff Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Eliminate graph rendering metadata lookups Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Batch DSStats metadata and last-value reads Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Batch interface speed metadata reads Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Update architectural hotspot baseline for RRD execution Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Batch RRD resolution profile lookups Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Batch DSStats summary aggregates Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Narrow phpseclib key types for the proxy encrypt and decrypt calls Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(rrd): bound the forked proxy peers so a stall cannot hang the suite The children accepted and read with no deadline while the parent used a 2s socket timeout, so any mismatch left the child blocked in read and the parent blocked in a bare pcntl_waitpid(). Note that accept() ignores SO_RCVTIMEO, so readiness has to be waited on with socket_select() rather than a socket option. Also corrects the array-command expectation, which still assumed develop's escape-everything behaviour rather than rrdtool_build_command() leaving the operation unquoted. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(rrd): seed proxy settings where the real read_config_option looks UnitStubs only declares its stub when lib/functions.php has not been loaded yet, so during a full run the real database-backed function is the one in effect and $unit_config_options was being ignored. The parent then dialled an address the child was not listening on, which is what stalled the handshake. Seeding the option cache keeps the settings visible to either implementation. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(rrd): assert the shell-free RRDtool launch instead of the old quoting __rrd_init now hands proc_open an argv array with bypass_shell, so escaping the binary path would put the quotes inside the filename. The assertion still expected the pre-argv line and had been failing unnoticed behind the suite hang. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(dsstats): derive the expected stamps instead of assuming UTC dsstats_poller_output() formats with date(), so the hardcoded UTC strings only matched on a UTC runner. The failure was masked by the suite hang. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(rrd): keep RRDtool control codes intact in stdin commands Escaping the backslash when converting a shell-quoted argument to an RRDtool token left legends such as " IO Wait\:" ending in a dangling control code, so RRDtool refused every graph carrying an escaped colon. Verified against RRDtool 1.7.2 in pipe mode; values with no unambiguous quoted form are now refused rather than mangled. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: use Cacti test bootstrap and config Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: resolve includes through Cacti path constants Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: match the escaping helpers by token, not by text lib/rrd.php documents its escaping behaviour in a comment, and the regex over raw source counted that as a call site. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh sink inventory after develop rebase Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(rrd): prove argv isolation and concurrent pipe draining Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(security): refresh sink inventory baseline after rebase onto develop Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
131 lines
6.3 KiB
PHP
131 lines
6.3 KiB
PHP
<?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 |
|
|
+-------------------------------------------------------------------------+
|
|
*/
|
|
|
|
/*
|
|
* __rrd_execute() states its contract in its own docblock: a caller passing a
|
|
* string quotes each variable argument with cacti_escapeshellarg() as it builds
|
|
* the line, because the assembled line is handed to shell_exec().
|
|
*
|
|
* These callers interpolated the RRDfile path instead. The paths come from
|
|
* data_template_data.data_source_path, which the web UI writes and an XML
|
|
* template import can also set, so the value crossed into a shell it was never
|
|
* quoted for. The affected core RRDtool integration files are checked here, so
|
|
* a new unsafe interpolation in those files fails before reaching a shell.
|
|
*/
|
|
|
|
$rrdSource = file_get_contents(CACTI_PATH_LIBRARY . '/rrd.php');
|
|
$boostSource = file_get_contents(CACTI_PATH_LIBRARY . '/boost.php');
|
|
$dsstatsSource = file_get_contents(CACTI_PATH_LIBRARY . '/dsstats.php');
|
|
$functionsSource = file_get_contents(CACTI_PATH_LIBRARY . '/functions.php');
|
|
|
|
foreach ([
|
|
'lib/rrd.php' => $rrdSource,
|
|
'lib/boost.php' => $boostSource,
|
|
'lib/dsstats.php' => $dsstatsSource,
|
|
'lib/functions.php' => $functionsSource,
|
|
] as $path => $source) {
|
|
if ($source === false) {
|
|
throw new RuntimeException("Unable to read $path for RRDtool argument tests.");
|
|
}
|
|
}
|
|
|
|
// --- the RRDtool binary itself ---
|
|
|
|
test('rrd_init launches the RRDtool binary without a shell', function () use ($rrdSource) {
|
|
// An argv array never reaches a shell, so the path must NOT be escaped here;
|
|
// quoting it would make the quotes part of the filename.
|
|
expect($rrdSource)->toContain("proc_open([\$path, '-'], \$descriptors, \$pipes, null, null, ['bypass_shell' => true]);");
|
|
expect($rrdSource)->not->toContain('popen(');
|
|
expect($rrdSource)->not->toContain("\$rrdtool = cacti_escapeshellcmd((string) read_config_option('path_rrdtool'));");
|
|
expect($rrdSource)->not->toContain("\$command = read_config_option('path_rrdtool') . ' - ';");
|
|
});
|
|
|
|
test('the proxy font path is quoted rather than hand wrapped', function () use ($rrdSource) {
|
|
expect($rrdSource)->toContain("'setenv RRD_DEFAULT_FONT ' . cacti_escapeshellarg(");
|
|
expect($rrdSource)->not->toContain('"setenv RRD_DEFAULT_FONT \'"');
|
|
});
|
|
|
|
// --- boost ---
|
|
|
|
test('boost quotes the RRDfile path on file_exists, last and create', function () use ($boostSource) {
|
|
expect($boostSource)->toContain("rrdtool_execute('last ' . cacti_escapeshellarg(\$rrd_path)");
|
|
expect($boostSource)->toContain("rrdtool_execute('create ' . cacti_escapeshellarg(\$data_source_path)");
|
|
expect($boostSource)->not->toContain('rrdtool_execute("file_exists $rrd_path"');
|
|
expect($boostSource)->not->toContain('rrdtool_execute("last $rrd_path"');
|
|
});
|
|
|
|
test('boost quotes the RRDfile path on update', function () use ($boostSource) {
|
|
expect($boostSource)->toContain("rrdtool_execute('update ' . cacti_escapeshellarg(\$rrd_path)");
|
|
expect($boostSource)->not->toContain('rrdtool_execute("update $rrd_path');
|
|
});
|
|
|
|
test('boost quotes the directory it asks RRDtool to create', function () use ($boostSource) {
|
|
expect($boostSource)->toContain("'is_dir ' . cacti_escapeshellarg(dirname(\$data_source_path))");
|
|
expect($boostSource)->toContain("'mkdir ' . cacti_escapeshellarg(dirname(\$data_source_path))");
|
|
});
|
|
|
|
// --- dsstats and functions ---
|
|
|
|
test('dsstats quotes the RRDfile path', function () use ($dsstatsSource) {
|
|
expect($dsstatsSource)->toContain("rrdtool_execute('file_exists ' . cacti_escapeshellarg(\$rrdfile)");
|
|
expect($dsstatsSource)->toContain("dsstats_rrdtool_execute('info ' . cacti_escapeshellarg(\$rrdfile)");
|
|
expect($dsstatsSource)->not->toContain('"info $rrdfile"');
|
|
});
|
|
|
|
test('the data source info reader quotes the RRDfile path', function () use ($functionsSource) {
|
|
expect($functionsSource)->toContain("rrdtool_execute('info ' . cacti_escapeshellarg(\$rrdfile)");
|
|
expect($functionsSource)->not->toContain('rrdtool_execute("info $rrdfile"');
|
|
});
|
|
|
|
// --- dump, restore, tune and resize ---
|
|
|
|
test('the RRDfile check quotes dump and restore paths', function () use ($rrdSource) {
|
|
expect($rrdSource)->toContain("'dump ' . cacti_escapeshellarg(\$file)");
|
|
expect($rrdSource)->toContain("'restore -f ' . cacti_escapeshellarg(\$xml_file) . ' ' . cacti_escapeshellarg(\$new_file)");
|
|
expect($rrdSource)->not->toContain('rrdtool_execute("dump $file"');
|
|
expect($rrdSource)->not->toContain('rrdtool_execute("restore -f $xml_file $file"');
|
|
expect($rrdSource)->not->toContain("\$xml_file = \$file . '.xml'");
|
|
});
|
|
|
|
test('tune and resize recommendations are quoted where they are built', function () use ($rrdSource) {
|
|
expect($rrdSource)->toContain("cacti_escapeshellarg(\$info['filename']) . ' --data-source-type '");
|
|
expect($rrdSource)->toContain("cacti_escapeshellarg(\$info['filename']) . ' --heartbeat '");
|
|
expect($rrdSource)->toContain("cacti_escapeshellarg(\$info['filename']) . ' --minimum '");
|
|
expect($rrdSource)->not->toContain("\$diff['tune'][] = \$info['filename'] . ' ' . '--data-source-type '");
|
|
});
|
|
|
|
// --- the sweep that catches a future caller ---
|
|
|
|
test('no RRDtool caller interpolates a bare path into the command line', function () use ($rrdSource, $boostSource, $dsstatsSource, $functionsSource) {
|
|
$offenders = [];
|
|
$pattern = '/rrdtool_execute\(\s*"[^"\r\n]*\$(rrd_path|rrdfile|data_source_path|xml_file|file)\b/';
|
|
|
|
expect(preg_match($pattern, 'rrdtool_execute("update --start -1h $rrdfile")'))->toBe(1);
|
|
|
|
foreach ([
|
|
'lib/rrd.php' => $rrdSource,
|
|
'lib/boost.php' => $boostSource,
|
|
'lib/dsstats.php' => $dsstatsSource,
|
|
'lib/functions.php' => $functionsSource
|
|
] as $name => $source) {
|
|
if (preg_match_all($pattern, $source, $m)) {
|
|
foreach ($m[0] as $hit) {
|
|
$offenders[] = $name . ': ' . $hit;
|
|
}
|
|
}
|
|
}
|
|
|
|
expect($offenders)->toBe([]);
|
|
});
|