Skip to content

Commit 6b5e617

Browse files
hardening: move to argument array pattern for shell execution (#6854)
1 parent 7f3af57 commit 6b5e617

File tree

3 files changed

+54
-2
lines changed

3 files changed

+54
-2
lines changed

lib/poller.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,17 @@ function exec_poll_php(string $command, bool $using_proc_function, array $pipes,
119119
*
120120
* @return void
121121
*/
122-
function exec_background(string $filename, string $args = '', string $redirect_args = '') : void {
122+
function exec_background(string $filename, string|array $args = '', string|array $redirect_args = '') : void {
123123
global $debug;
124124

125+
if (is_array($args)) {
126+
$args = implode(' ', array_map('cacti_escapeshellarg', $args));
127+
}
128+
129+
if (is_array($redirect_args)) {
130+
$redirect_args = implode(' ', $redirect_args); // redirect args should not be shell escaped generally as they contain > etc
131+
}
132+
125133
cacti_log("DEBUG: About to Spawn a Remote Process [CMD: $filename, ARGS: $args]", true, 'POLLER', ($debug ? POLLER_VERBOSITY_NONE : POLLER_VERBOSITY_DEBUG));
126134

127135
if ($filename != '') {

lib/rrd.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,9 +305,13 @@ function rrdtool_execute() : mixed {
305305
return call_user_func_array($function, $args);
306306
}
307307

308-
function __rrd_execute(string $command_line, bool $log_to_stdout, int $output_flag = RRDTOOL_OUTPUT_STDOUT, mixed $rrdtool_pipe = null, string $logopt = 'WEBLOG') : mixed {
308+
function __rrd_execute(string|array $command_line, bool $log_to_stdout, int $output_flag = RRDTOOL_OUTPUT_STDOUT, mixed $rrdtool_pipe = null, string $logopt = 'WEBLOG') : mixed {
309309
static $last_command;
310310

311+
if (is_array($command_line)) {
312+
$command_line = implode(' ', array_map('cacti_escapeshellarg', $command_line));
313+
}
314+
311315
/**
312316
* WIN32: before sending this command off to rrdtool, get rid
313317
* of all of the backslash (\) characters. Unix does not care; win32 does.
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
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+
require_once dirname(__DIR__) . '/Helpers/CactiStubs.php';
16+
17+
// Mock cacti_escapeshellarg if it doesn't exist in testing env
18+
if (!function_exists('cacti_escapeshellarg')) {
19+
function cacti_escapeshellarg($arg) {
20+
return "'" . str_replace("'", "'\\''", $arg) . "'";
21+
}
22+
}
23+
24+
test('__rrd_execute correctly handles array of arguments', function () {
25+
$args = ['graph', '-', '--start', '12345; id'];
26+
27+
// Simulated logic from __rrd_execute
28+
$command_line = implode(' ', array_map('cacti_escapeshellarg', $args));
29+
30+
expect($command_line)->toBe("'graph' '-' '--start' '12345; id'");
31+
});
32+
33+
test('exec_background correctly handles array of arguments', function () {
34+
$args = ['--poller=1', '--network=192.168.1.0/24; id'];
35+
36+
// Simulated logic from exec_background
37+
$args_string = implode(' ', array_map('cacti_escapeshellarg', $args));
38+
39+
expect($args_string)->toBe("'--poller=1' '--network=192.168.1.0/24; id'");
40+
});

0 commit comments

Comments
 (0)