From bc31ed0e4647a568a1596cc2e09664675479a70d Mon Sep 17 00:00:00 2001
From: Pim van den Berg
Date: Fri, 1 Jan 2010 19:36:31 +0100
Subject: better validation of possible user input (get)

---
 detail.php            | 11 ++++++-----
 graph.php             | 11 +++++++----
 host.php              |  4 ++--
 inc/collectd.inc.php  |  2 ++
 inc/functions.inc.php | 27 +++++++++++++++++++++++++++
 inc/html.inc.php      | 13 ++++++++-----
 plugin.php            |  5 +++--
 7 files changed, 55 insertions(+), 18 deletions(-)
 create mode 100644 inc/functions.inc.php

diff --git a/detail.php b/detail.php
index e10fae3..d153768 100644
--- a/detail.php
+++ b/detail.php
@@ -1,6 +1,7 @@
 <?php
 
 require_once 'conf/common.inc.php';
+require_once 'inc/functions.inc.php';
 require_once 'inc/html.inc.php';
 require_once 'inc/collectd.inc.php';
 
@@ -10,11 +11,11 @@ if (empty($_GET['x']))
 if (empty($_GET['y']))
 	$_GET['y'] = $CONFIG['detail-heigth'];
 
-$host = $_GET['h'];
-$plugin = $_GET['p'];
-$pinstance = $_GET['pi'];
-$type = $_GET['t'];
-$tinstance = $_GET['ti'];
+$host = validate_get($_GET['h'], 'host');
+$plugin = validate_get($_GET['p'], 'plugin');
+$pinstance = validate_get($_GET['pi'], 'pinstance');
+$type = validate_get($_GET['t'], 'type');
+$tinstance = validate_get($_GET['ti'], 'tinstance');
 $width = $_GET['x'];
 $heigth = $_GET['y'];
 $seconds = $_GET['s'];
diff --git a/graph.php b/graph.php
index 77bfcb6..effecef 100644
--- a/graph.php
+++ b/graph.php
@@ -1,21 +1,24 @@
 <?php
 
 require_once 'conf/common.inc.php';
+require_once 'inc/functions.inc.php';
 
+$plugin = validate_get($_GET['p'], 'plugin');
 $width = empty($_GET['x']) ? $CONFIG['width'] : $_GET['x'];
 $heigth = empty($_GET['y']) ? $CONFIG['heigth'] : $_GET['y'];
 
-if (!preg_match('/^[a-z]+$/', $_GET['p'])) {
+if (validate_get($_GET['h'], 'host') === NULL) {
 	die_img('Error: plugin contains unknown characters.');
 	exit;
 }
 
-if (!file_exists($CONFIG['webdir'].'/plugin/'.$_GET['p'].'.php')) {
-	die_img(sprintf('Error: plugin not available (%s).', $_GET['p']));
+if (!file_exists($CONFIG['webdir'].'/plugin/'.$plugin.'.php')) {
+	die_img(sprintf('Error: plugin not available (%s).', $plugin));
 	exit;
 }
 
-include $CONFIG['webdir'].'/plugin/'.$_GET['p'].'.php';
+# load plugin
+include $CONFIG['webdir'].'/plugin/'.$plugin.'.php';
 
 
 function die_img($msg) {
diff --git a/host.php b/host.php
index 42179f3..7dd509a 100644
--- a/host.php
+++ b/host.php
@@ -4,8 +4,8 @@ require_once 'conf/common.inc.php';
 require_once 'inc/html.inc.php';
 require_once 'inc/collectd.inc.php';
 
-$host = $_GET['h'];
-$splugin = $_GET['p'];
+$host = validate_get($_GET['h'], 'host');
+$splugin = validate_get($_GET['p'], 'plugin');
 
 html_start();
 
diff --git a/inc/collectd.inc.php b/inc/collectd.inc.php
index 2a44fba..ab4340f 100644
--- a/inc/collectd.inc.php
+++ b/inc/collectd.inc.php
@@ -1,5 +1,7 @@
 <?php
 
+# collectd related functions
+
 require_once 'conf/common.inc.php';
 
 # returns an array of all collectd hosts
diff --git a/inc/functions.inc.php b/inc/functions.inc.php
new file mode 100644
index 0000000..2906530
--- /dev/null
+++ b/inc/functions.inc.php
@@ -0,0 +1,27 @@
+<?php
+
+# global functions
+
+function validate_get($value, $type) {
+	switch($type) {
+		case 'host':
+			if (!preg_match('/^[\d\w\W]+$/', $value))
+				return NULL;
+		break;
+		case 'plugin':
+		case 'type':
+			if (!preg_match('/^\w+$/', $value))
+				return NULL;
+		break;
+		case 'pinstance':
+		case 'tinstance':
+			if (!preg_match('/^[\d\w-]+$/', $value))
+				return NULL;
+		break;
+	}
+
+	return $value;
+}
+
+
+?>
diff --git a/inc/html.inc.php b/inc/html.inc.php
index e93cc66..e38ca8e 100644
--- a/inc/html.inc.php
+++ b/inc/html.inc.php
@@ -1,7 +1,10 @@
 <?php
 
+# html related functions
+
 require_once 'conf/common.inc.php';
 require_once 'inc/rrdtool.class.php';
+require_once 'inc/functions.inc.php';
 require_once 'inc/collectd.inc.php';
 
 function html_start() {
@@ -78,15 +81,15 @@ function host_summary($hosts) {
 
 
 function breadcrumbs() {
-	if (isset($_GET['h']))
+	if (validate_get($_GET['h'], 'host'))
 		$path = ' - '.ucfirst($_GET['h']);
-	if (isset($_GET['p']))
+	if (validate_get($_GET['p'], 'plugin'))
 		$path .= ' - '.ucfirst($_GET['p']);
-	if (isset($_GET['pi']))
+	if (validate_get($_GET['pi'], 'pinstance'))
 		$path .= ' - '.$_GET['pi'];
-	if (isset($_GET['t']) && isset($_GET['p']) && $_GET['t'] != $_GET['p'])
+	if (validate_get($_GET['t'], 'type') && validate_get($_GET['p'], 'plugin') && $_GET['t'] != $_GET['p'])
 		$path .= ' - '.$_GET['t'];
-	if (isset($_GET['ti']))
+	if (validate_get($_GET['ti'], 'tinstance'))
 		$path .= ' - '.$_GET['ti'];
 
 	return $path;
diff --git a/plugin.php b/plugin.php
index 054c559..54f75fb 100644
--- a/plugin.php
+++ b/plugin.php
@@ -1,10 +1,11 @@
 <?php
 
 require_once 'conf/common.inc.php';
+require_once 'inc/functions.inc.php';
 require_once 'inc/html.inc.php';
 
-$host = $_GET['h'];
-$plugin = $_GET['p'];
+$host = validate_get($_GET['h'], 'host');
+$plugin = validate_get($_GET['p'], 'plugin');
 
 if ($_GET['a'] == 'del') {
 	plugin_header($host, $plugin, 1);
-- 
cgit v1.1