security: escape path_php_binary in test_data_source (#7786)

This commit is contained in:
Thomas Vincent authored and GitHub committed 2026-08-22 17:14:12 -07:00
1 parent fb8e39684e
commit 02386f5006
3 files changed
+55 -2

No files matched your search

+1
View File
@@ -103,6 +103,7 @@ Cacti CHANGELOG
-issue#7732: Bind device IDs and escape graph and device HTML sinks
-issue#7715: Refuse to serve the REST scaffold unless it is explicitly enabled
-issue#7751: Declare the translation helper before the disabled-i18n fallback returns
-issue#7469: Escape path_php_binary before it reaches shell_exec in test_data_source
-feature#412: Import Export for Graph and Tree rules
-feature#1214: Move Tree Create/Remove/Modify Functions to lib/api_tree.php
-feature#1361: UI should update alternate row colors
+5 -2
View File
@@ -2647,7 +2647,10 @@ function test_data_source(int $data_template_id, int $host_id, int $snmp_query_i
$output = shell_exec($script_path);
} else {
// Script server is a bit more complicated
$php = read_config_option('path_php_binary');
// path_php_binary is admin-set and reaches shell_exec() below; escape
// it so a shell metacharacter cannot inject a command (issue#7469,
// forward-port of the release/1.2.31 fix).
$php = cacti_escapeshellcmd(read_config_option('path_php_binary'));
$parts = explode(' ', $script_path);
dsv_log('parts', $parts);
@@ -2939,7 +2942,7 @@ function test_data_source(int $data_template_id, int $host_id, int $snmp_query_i
$prepend = $script_queries['arg_prepend'];
}
$script_path = read_config_option('path_php_binary') . ' -q ' . get_script_query_path(trim($prepend . ' ' . $script_queries['arg_get'] . ' ' . $identifier . ' "' . $snmp_index . '"'), $script_queries['script_path'], $host_id);
$script_path = cacti_escapeshellcmd(read_config_option('path_php_binary')) . ' -q ' . get_script_query_path(trim($prepend . ' ' . $script_queries['arg_get'] . ' ' . $identifier . ' "' . $snmp_index . '"'), $script_queries['script_path'], $host_id);
} else {
$action = POLLER_ACTION_SCRIPT;
$script_path = get_script_query_path(trim(($script_queries['arg_prepend'] ?? '') . ' ' . $script_queries['arg_get'] . ' ' . $identifier . ' "' . $snmp_index . '"'), $script_queries['script_path'], $host_id);
@@ -0,0 +1,49 @@
<?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 |
+-------------------------------------------------------------------------+
*/
/*
* Regression test for issue#7469.
*
* test_data_source()'s two script-server branches interpolated
* path_php_binary, an admin-set Settings > Paths value, into a command handed
* to shell_exec(). release/1.2.31 wrapped it in cacti_escapeshellcmd(); the
* fix never reached develop, leaving authenticated command injection in both
* the PHP_SCRIPT_SERVER and QUERY_SCRIPT_SERVER paths.
*/
require_once dirname(__DIR__, 3) . '/Helpers/CactiStubs.php';
require_once dirname(__DIR__, 4) . '/include/global.php';
$source = file_get_contents(__DIR__ . '/../../../../lib/functions.php');
test('the script-server branch escapes path_php_binary', function () use ($source) {
expect($source)->toContain("\$php = cacti_escapeshellcmd(read_config_option('path_php_binary'));")
->and($source)->not->toContain("\$php = read_config_option('path_php_binary');");
});
test('the query-script-server branch escapes path_php_binary', function () use ($source) {
expect($source)->toContain("\$script_path = cacti_escapeshellcmd(read_config_option('path_php_binary')) . ' -q '")
->and($source)->not->toContain("\$script_path = read_config_option('path_php_binary') . ' -q '");
});
test('escaping neutralizes a shell metacharacter in path_php_binary', function () {
$evil = 'php ; touch /tmp/pwned ; echo';
$cmd = cacti_escapeshellcmd($evil) . ' -q /tmp/script.php';
// escapeshellcmd backslash-escapes the ; so it can no longer start a
// new command; the injected touch never runs as its own statement.
expect($cmd)->not->toMatch('/(^|[^\\\\]);\s*touch/')
->and($cmd)->toContain('\;');
});