no input validations in ss_get_mysql_stats.php parse_cmdline

[dbmiller]

Have an installation of Cacti which upgraded from 0.8.8h to v1.0.4 with monitor plugins 1.1.7 that stopped pulling data after the upgrade. Traced the lack of data to the poller call to ss_get_mysql_stats.php. Results from the script logging:

2017-04-07 17:21:04 at /var/lib/cacti/scripts/ss_get_mysql_stats.php:71
'Found configuration file /etc/cacti/ss_get_mysql_stats.php.cnf'
2017-04-07 17:21:04 at /var/lib/cacti/scripts/ss_get_mysql_stats.php:126
array (
0 => '/usr/share/cacti/scripts/ss_get_mysql_stats.php',
1 => '--host',
2 => 'x.x.x.x', //redacted to protect the guilty
3 => '--items',
4 => 'ju,jv,jw,jx,jy,jz,kg,kh,ki,kj,kk',
5 => '--user',
6 => '',
7 => '--pass',
8 => '',
9 => '--port',
10 => '',
11 => '--server-id',
12 => '',
)
2017-04-07 17:21:04 parse_cmdline() at /var/lib/cacti/scripts/ss_get_mysql_stats.php:239
array (
'host' => 'x.x.x.x', //redacted to protect the guilty
'items' => 'ju,jv,jw,jx,jy,jz,kg,kh,ki,kj,kk',
'user' => '',
'pass' => '',
'port' => '',
'server-id' => '',
)
2017-04-07 17:21:04 ss_get_mysql_stats() at /var/lib/cacti/scripts/ss_get_mysql_stats.php:266
'Cache file is /tmp/x.x.x.x-mysql_cacti_stats.txt:' //redacted to protect the guilty
2017-04-07 17:21:04 ss_get_mysql_stats() at /var/lib/cacti/scripts/ss_get_mysql_stats.php:283
'The cache file seems too small or stale'
2017-04-07 17:21:04 ss_get_mysql_stats() at /var/lib/cacti/scripts/ss_get_mysql_stats.php:316
array (
0 => 'Connecting to',
1 => 'x.x.x.x', //redacted to protect the guilty
2 => '',
3 => '',
4 => '',
)

Looks like the post upgrade version is passing empty strings as arguments when they are not explicitly set in the interface where previous versions would pass NULL. The set empty strings then don't permit the variables from the script config file to take precedence. Propose adding input validation to parse_cmdline as long as there isn't any real expectation to ever need to actually use an empty string for one of the script parameters. Workaround code from my instance:

function parse_cmdline( $args ) {
   $options = array();
   while (list($tmp, $p) = each($args)) {
      if (strpos($p, '--') === 0) {
         $param = substr($p, 2);
         $value = null;
         $nextparam = current($args);
         if ($nextparam !== false && strpos($nextparam, '--') !==0) {
            list($tmp, $value) = each($args);
         }
         if ($value != NULL){
            if ($value != ''){
               $options[$param] = $value;
            }
         }
      }
   }
   if ( array_key_exists('host', $options) ) {
      $options['host'] = substr($options['host'], 0, 4) == 'tcp:' ? substr($options['host'], 4) : $options['host'];
   }
   debug($options);
   return $options;
}

Results from logging after validation:

2017-04-07 17:51:04 at /var/lib/cacti/scripts/ss_get_mysql_stats.php:71
'Found configuration file /etc/cacti/ss_get_mysql_stats.php.cnf'
2017-04-07 17:51:04 at /var/lib/cacti/scripts/ss_get_mysql_stats.php:126
array (
0 => '/usr/share/cacti/scripts/ss_get_mysql_stats.php',
1 => '--host',
2 => 'x.x.x.x', //redacted to protect the guilty
3 => '--items',
4 => 'ju,jv,jw,jx,jy,jz,kg,kh,ki,kj,kk',
5 => '--user',
6 => '',
7 => '--pass',
8 => '',
9 => '--port',
10 => '',
11 => '--server-id',
12 => '',
)
2017-04-07 17:51:04 parse_cmdline() at /var/lib/cacti/scripts/ss_get_mysql_stats.php:241
array (
'host' => 'x.x.x.x', //redacted to protect the guilty
'items' => 'ju,jv,jw,jx,jy,jz,kg,kh,ki,kj,kk',
)
2017-04-07 17:51:04 ss_get_mysql_stats() at /var/lib/cacti/scripts/ss_get_mysql_stats.php:268
'Cache file is /tmp/x.x.x.x-mysql_cacti_stats.txt' //redacted to protect the guilty
2017-04-07 17:51:04 ss_get_mysql_stats() at /var/lib/cacti/scripts/ss_get_mysql_stats.php:285
'The cache file seems too small or stale'
2017-04-07 17:51:04 ss_get_mysql_stats() at /var/lib/cacti/scripts/ss_get_mysql_stats.php:318
array (
0 => 'Connecting to',
1 => 'x.x.x.x', //redacted to protect the guilty
2 => 3306,
3 => 'user', //redacted to protect the guilty
4 => 'password', //redacted to protect the guilty
)

I am not quite sure if the change in how the unset value is put into the input method is intentional with the release or a result of an upgrade to the cacti database when upgrading the server, but it may be a good idea to put the input validation in anyways. I might do some testing with a fresh install when I have some more time to see if it was the result of the upgrade.

[deniskin82]

Fix maybe in pull request #48

[lukatendai]

Fix in pull request #48 did the job. I put manually all the changes from that pull and now it works. Thank you!