From 4a737bc1abdbef7e0698b006704a26583a4c61df Mon Sep 17 00:00:00 2001
From: Peter Wu
Date: Sun, 20 Jul 2014 23:30:49 +0200
Subject: Use a more secure command line building method
Previously, a command is built by string concatenation. Here, the
distinction between a value and multiple params got lost. Solve this
by using an array for shell arguments. As the escaping is now removed
from the `rrd_gen_graph` function, the canvas style needs to manually
add those quotes to make the JS code still work. That only supports
double-quotes, so hopefully nobody creates a name with a double quote
as that would break the fragile JS command line parser.
Separate the rrdtool options from the rrdtool graph command to make the
`$graph_type == 'canvas'` option work (it would otherwise not understand
the `rrdtool graph - -a PNG` option).
Merge the SVG and PNG cases as they are the same except for the
Content-Type header.
Fix a missing html escape in a debug style.
---
conf/config.php | 4 +-
graph.php | 11 ++++-
plugin/cpu.json | 2 +-
type/Base.class.php | 104 +++++++++++++++++++++++++-----------------
type/Default.class.php | 10 ++--
type/GenericIO.class.php | 20 ++++----
type/GenericStacked.class.php | 10 ++--
type/Uptime.class.php | 12 ++---
8 files changed, 100 insertions(+), 73 deletions(-)
diff --git a/conf/config.php b/conf/config.php
index 30eaaf9..57a0188 100644
--- a/conf/config.php
+++ b/conf/config.php
@@ -12,8 +12,8 @@ $CONFIG['typesdb'][] = '/usr/share/collectd/types.db';
# rrdtool executable
$CONFIG['rrdtool'] = '/usr/bin/rrdtool';
-# rrdtool special options
-$CONFIG['rrdtool_opts'] = '';
+# rrdtool special command-line options
+$CONFIG['rrdtool_opts'] = [];
# category of hosts to show on main page
#$CONFIG['cat']['category1'] = array('host1', 'host2');
diff --git a/graph.php b/graph.php
index 1727c1b..3e33b92 100644
--- a/graph.php
+++ b/graph.php
@@ -86,7 +86,16 @@ if (isset($plugin_json[$type]['vertical'])) {
}
if (isset($plugin_json[$type]['rrdtool_opts'])) {
- $obj->rrdtool_opts[] = $plugin_json[$type]['rrdtool_opts'];
+ $rrdtool_extra_opts = $plugin_json[$type]['rrdtool_opts'];
+ # compatibility with plugins which specify arguments as string
+ if (is_string($rrdtool_extra_opts)) {
+ $rrdtool_extra_opts = explode(' ', $rrdtool_extra_opts);
+ }
+
+ $obj->rrdtool_opts = array_merge(
+ $obj->rrdtool_opts,
+ $rrdtool_extra_opts
+ );
}
if (isset($plugin_json[$type]['datasize']) and $plugin_json[$type]['datasize'])
diff --git a/plugin/cpu.json b/plugin/cpu.json
index 50254f8..34e021d 100644
--- a/plugin/cpu.json
+++ b/plugin/cpu.json
@@ -2,7 +2,7 @@
"cpu": {
"title": "CPU-{{PI}} usage",
"vertical": "Jiffies",
- "rrdtool_opts": "-u 100",
+ "rrdtool_opts": ["-u", "100"],
"type": "stacked",
"legend": {
"idle": {
diff --git a/type/Base.class.php b/type/Base.class.php
index 1f42f4f..72ce437 100644
--- a/type/Base.class.php
+++ b/type/Base.class.php
@@ -36,8 +36,14 @@ class Type_Base {
function __construct($config, $_get) {
$this->datadir = $config['datadir'];
$this->rrdtool = $config['rrdtool'];
- if (!empty($config['rrdtool_opts']))
- $this->rrdtool_opts[] = $config['rrdtool_opts'];
+ if (!empty($config['rrdtool_opts'])) {
+ if (is_array($config['rrdtool_opts'])) {
+ $this->rrdtool_opts = $config['rrdtool_opts'];
+ } else {
+ $this->rrdtool_opts = explode(' ',
+ $config['rrdtool_opts']);
+ }
+ }
$this->cache = $config['cache'];
$this->parse_get($_get);
$this->rrd_title = sprintf(
@@ -200,78 +206,90 @@ class Type_Base {
$this->rainbow_colors();
$this->colors = $colors + $this->colors;
- $graphdata = $this->rrd_gen_graph();
+ $graphoptions = $this->rrd_gen_graph();
+ # $shellcmd contains escaped rrdtool arguments
+ $shellcmd = array();
+ foreach ($graphoptions as $arg)
+ $shellcmd[] = escapeshellarg($arg);
$style = $debug !== false ? $debug : $this->graph_type;
switch ($style) {
case 'cmd':
print '';
- foreach ($graphdata as $d) {
+ foreach ($shellcmd as $d) {
printf("%s \\\n", htmlentities($d));
}
print '';
break;
case 'canvas':
- printf('