From c7283e1cae6bbdc13464a77ae7f22bd9489fe089 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Sun, 20 Jul 2014 23:23:17 +0200 Subject: Defensive programming: more urlencode/htmlentities Make build_url return an URL, not HTML. This separates presentation from data. plugin_header's return value is unused, remove the unnecessary return. At places where `printf("", $x);` is used, it is now converted to `printf("", htmlentities($x));` since the single quote is not escaped by default by htmlentities. In case the canvas style is used, JS should use `textContent` instead of `innerHTML` to avoid reading `"` instead of `"`. Nobody (should) use(s) IE6 anymore, so it is a safe change. While at it, use the standard charset attribute of meta to specify the character set (UTF-8). --- detail.php | 9 +++-- inc/html.inc.php | 98 ++++++++++++++++++++++++++++------------------------- js/CGP.js | 2 +- type/Base.class.php | 4 +-- 4 files changed, 61 insertions(+), 52 deletions(-) diff --git a/detail.php b/detail.php index 4fb43b0..8f5a845 100644 --- a/detail.php +++ b/detail.php @@ -46,7 +46,10 @@ foreach($CONFIG['term'] as $key => $s) { $args['s'] = $s; $selected = selected_timerange($seconds, $s); printf('
  • %s
    • '."\n", - $selected, $CONFIG['weburl'], build_url('detail.php', $args), $key); + $selected, + htmlentities($CONFIG['weburl']), + htmlentities(build_url('detail.php', $args)), + htmlentities($key)); } print "\n"; @@ -54,7 +57,9 @@ if ($CONFIG['graph_type'] == 'canvas') { chdir($CONFIG['webdir']); include $CONFIG['webdir'].'/graph.php'; } else { - printf(''."\n", $CONFIG['weburl'], build_url('graph.php', $_GET)); + printf("\n", + htmlentities($CONFIG['weburl']), + htmlentities(build_url('graph.php', $_GET))); } echo ''; echo "\n"; diff --git a/inc/html.inc.php b/inc/html.inc.php index 69609d8..b8d2f82 100644 --- a/inc/html.inc.php +++ b/inc/html.inc.php @@ -11,14 +11,15 @@ function html_start() { global $CONFIG; $path = htmlentities(breadcrumbs()); + $html_weburl = htmlentities($CONFIG['weburl']); echo << - + CGP{$path} - + EOT; @@ -31,16 +32,16 @@ EOT; if ($CONFIG['graph_type'] == 'canvas') { echo << - - - - - - - - - + + + + + + + + + + EOT; } @@ -50,7 +51,7 @@ echo <<
    @@ -74,27 +75,29 @@ function html_end() { $version = 'v'.$version[0]; } + $html_weburl = htmlentities($CONFIG['weburl']); + echo << EOT; if ($CONFIG['graph_type'] == 'canvas') { echo << + EOT; if ($CONFIG['rrd_fetch_method'] == 'async') { echo << + EOT; } else { echo << + EOT; } @@ -109,7 +112,11 @@ EOT; function plugin_header($host, $plugin) { global $CONFIG; - return printf("

    %s

    \n", $CONFIG['weburl'], $host, $plugin, $plugin); + printf("

    %s

    \n", + htmlentities($CONFIG['weburl']), + urlencode($host), + urlencode($plugin), + htmlentities($plugin)); } function plugins_list($host, $selected_plugins = array()) { @@ -121,20 +128,21 @@ function plugins_list($host, $selected_plugins = array()) { echo '

    Plugins

    '; echo '
      '; - printf("
    • overview
      • \n", + printf("
    • overview
      • \n", selected_overview($selected_plugins), - $CONFIG['weburl'], - $host + htmlentities($CONFIG['weburl']), + urlencode($host) ); # first the ones defined as ordered foreach($CONFIG['overview'] as $plugin) { if (in_array($plugin, $plugins)) { - printf("
    • %4\$s
      • \n", + printf("
    • %s
      • \n", selected_plugin($plugin, $selected_plugins), - $CONFIG['weburl'], - $host, - $plugin + htmlentities($CONFIG['weburl']), + urlencode($host), + urlencode($plugin), + htmlentities($plugin) ); } } @@ -142,11 +150,12 @@ function plugins_list($host, $selected_plugins = array()) { # other plugins foreach($plugins as $plugin) { if (!in_array($plugin, $CONFIG['overview'])) { - printf("
    • %4\$s
      • \n", + printf("
    • %s
      • \n", selected_plugin($plugin, $selected_plugins), - $CONFIG['weburl'], - $host, - $plugin + htmlentities($CONFIG['weburl']), + urlencode($host), + urlencode($plugin), + htmlentities($plugin) ); } } @@ -181,8 +190,8 @@ function host_summary($cat, $hosts) { $rrd = new RRDTool($CONFIG['rrdtool']); - printf('
      ', $cat); - printf('%s', $cat); + printf('
      ', htmlentities($cat)); + printf('%s', htmlentities($cat)); echo "\n"; $row_style = array(0 => "even", 1 => "odd"); @@ -193,7 +202,9 @@ function host_summary($cat, $hosts) { printf('', $row_style[$host_counter % 2]); printf('', - $CONFIG['weburl'],$host, $host); + htmlentities($CONFIG['weburl']), + urlencode($host), + htmlentities($host)); if ($CONFIG['showload']) { require_once 'type/Default.class.php'; @@ -308,11 +319,10 @@ function graphs_from_plugin($host, $plugin, $overview=false) { $_GET['s'] = $time; include $CONFIG['webdir'].'/graph.php'; } else { - printf(''."\n", - $CONFIG['weburl'], - build_url('detail.php', $items, $time), - $CONFIG['weburl'], - build_url('graph.php', $items, $time) + printf(''."\n", + htmlentities($CONFIG['weburl']), + htmlentities(build_url('detail.php', $items, $time)), + htmlentities(build_url('graph.php', $items, $time)) ); } } @@ -328,17 +338,11 @@ function build_url($base, $items, $s=NULL) { if (!is_numeric($s)) $s = $CONFIG['time_range']['default']; - $i=0; - foreach ($items as $key => $value) { - # don't include empty values - if ($value == 'NULL') - continue; + // Remove all empty values + $items = array_filter($items, 'strlen'); - $base .= sprintf('%s%s=%s', $i==0 ? '?' : '&', $key, $value); - $i++; - } if (!isset($items['s'])) - $base .= '&s='.$s; + $items['s'] = $s; - return $base; + return "$base?" . http_build_query($items, '', '&'); } diff --git a/js/CGP.js b/js/CGP.js index bbfd1b5..c30cfd2 100644 --- a/js/CGP.js +++ b/js/CGP.js @@ -66,7 +66,7 @@ function prepare_draw(id) { RrdGraph.prototype.mousex = 0; RrdGraph.prototype.mousedown = false; - var cmdline = document.getElementById(id).innerHTML; + var cmdline = document.getElementById(id).textContent; var gfx = new RrdGfxCanvas(id); var fetch = new RrdDataFile(); var rrdcmdline = null; diff --git a/type/Base.class.php b/type/Base.class.php index b74c1b1..ff9a235 100644 --- a/type/Base.class.php +++ b/type/Base.class.php @@ -203,14 +203,14 @@ class Type_Base { case 'cmd': print '
      ';
 				foreach ($graphdata as $d) {
-					printf("%s \\\n", $d);
+					printf("%s \\\n", htmlentities($d));
 				}
 				print '
      '; break; case 'canvas': printf('', sha1(serialize($graphdata))); foreach ($graphdata as $d) { - printf("%s\n", $d); + printf("%s\n", htmlentities($d)); } print ''; break; -- cgit v1.1
      %s