CVE-2013-1770: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Cross-site scripting (XSS) vulnerability in views_view.php in Ganglia Web 3.5.7 allows remote attackers to inject arbitrary web script or HTML via the view_name parameter.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
Published: 2014-04-02T16:05Z
Repository: https://github.com/ganglia/ganglia-web
Language: PHP
Files changed: functions.php, get_context.php, views_view.php
Commit date: 2013-03-06 12:40:15 -0500
Commit message: Add limitation for view names. Fixes #160

Diff: diff --git a/functions.php b/functions.php
index 7aa93b6..d288911 100644
--- a/functions.php
+++ b/functions.php
@@ -363,10 +363,22 @@ function is_valid_hex_color( $string )
   }
   return $return_value;
     
 }
 
+#------------------------------------------------------------------------------
+# Allowed view name characters are alphanumeric plus space, dash and underscore
+function is_proper_view_name( $string )
+{
+  if(preg_match("/[^a-zA-z0-9_\-\ ]/", $string)){
+    return false;
+  } else {
+    return true;
+  }
+}
+
+
 #------------------------------------------------------------------------------
 # Return a shortened version of a FQDN
 # if "hostname" is numeric only, assume it is an IP instead
 # 
 function strip_domainname( $hostname ) {
diff --git a/get_context.php b/get_context.php
index c7c3baf..97f9e46 100644
--- a/get_context.php
+++ b/get_context.php
@@ -13,12 +13,14 @@ $cluster_designator = "Cluster Overview";
 $user['clustername'] = isset($_GET["c"]) ?
     escapeshellcmd( clean_string( rawurldecode($_GET["c"]) ) ) : NULL;
 $user['gridname'] = isset($_GET["G"]) ?
     escapeshellcmd( clean_string( rawurldecode($_GET["G"]) ) ) : NULL;
 
-$user['viewname'] = isset($_GET["vn"]) ?
-    escapeshellcmd( clean_string( rawurldecode($_GET["vn"]) ) ) : '';
+$user['viewname'] = '';
+if ( isset($_GET["vn"]) &&  is_proper_view_name ($_GET["vn"]) ) {
+    $user['viewname'] = $_GET["vn"];
+}
 
 if($conf['case_sensitive_hostnames'] == 1) {
     $user['hostname'] = isset($_GET["h"]) ?
         escapeshellcmd( clean_string( rawurldecode($_GET["h"]) ) ) : NULL;
 } else {
diff --git a/views.php b/views.php
deleted file mode 100644
index a99e930..0000000
--- a/views.php
+++ /dev/null
@@ -1,55 +0,0 @@
-<?php
-
-include_once("./eval_conf.php");
-include_once("./functions.php");
-
-if( ! checkAccess(GangliaAcl::ALL_VIEWS, GangliaAcl::VIEW, $conf) ) {
-  die("You do not have access to view views.");
-}
-
-// Load the metric caching code we use if we need to display graphs
-retrieve_metrics_cache();
-
-$base = isset($_GET['base']) ? rawurlencode($_GET['base']) . "/" : "";
-?>
-
-<html>
-<head>
-<script type="text/javascript" src="<?php print $base; ?>js/jquery-1.9.1.min.js"></script>
-<script type="text/javascript" src="<?php print $base; ?>js/jquery-ui-1.9.1.custom.min.js"></script>
-<link type="text/css" href="css/smoothness/jquery-ui-1.9.1.custom.min.css" rel="stylesheet"/>
-<link rel="stylesheet" href="./styles.css" type="text/css">
-</head>
-<body>
-  <div>
-
-<?php
-  $available_views = get_available_views();
-  $view_name = $_GET['vn'];
-
-  // Let's find the view definition
-  foreach ($available_views as $view_id => $view) {
-    if ($view['view_name'] == $view_name) {
-      $view_elements = get_view_graph_elements($view);
-      $range_args = "";
-      if (isset($_GET['r']) && $_GET['r'] != "") 
-	$range_args .= "&amp;r=" . rawurlencode($_GET['r']);
-      if (isset($_GET['cs']) && isset($_GET['ce'])) 
-	$range_args .= "&amp;cs=" . rawurlencode($_GET['cs']) . "&amp;ce=" . rawurlencode($_GET['ce']);
-
-      if (count($view_elements) != 0) {
-	foreach ($view_elements as $id => $element) {
-	  $legend = isset($element['hostname']) ? $element['hostname'] : "Aggregate graph";
-          $base = isset($_GET['base']) ? rawurlencode($_GET['base']) : '.';
-	  print "<a href=\"" . $base . "/graph_all_periods.php?" . htmlentities($element['graph_args']) ."&amp;z=large\"><img title=\"" . $legend . " - " . $element['name'] . "\" border=0 src=\"" . $base . "/graph.php?" . htmlentities($element['graph_args']) . "&amp;z=small" . $range_args .  "\" style=\"padding:2px;\"></a>";
-	}
-      } else {
-	print "No graphs defined for this view. Please add some";
-      }
-    }
-  } 
-?>
-  </div>
-</body>
-</html>
-
diff --git a/views_view.php b/views_view.php
index 3ec6f5a..5ab6e69 100644
--- a/views_view.php
+++ b/views_view.php
@@ -4,10 +4,24 @@ include_once("./functions.php");
 include_once("./global.php");
 
 if (! checkAccess(GangliaAcl::ALL_VIEWS, GangliaAcl::VIEW, $conf))
   die("You do not have access to view views.");
 
+$user['view_name'] = $_REQUEST['view_name'];
+
+if( !is_proper_view_name ( $user['view_name'])){
+?>
+<div class="ui-widget">
+  <div class="ui-state-default ui-corner-all" styledefault="padding: 0 .7em;"> 
+    <p><span class="ui-icon ui-icon-alert" style="float: left; margin-right: .3em;"></span> 
+    View names valid characters are 0-9, a-z, A-Z, -, _ and space. View has not been created.</p>
+  </div>
+</div>
+<?php
+  exit(0);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // Create new view
 ///////////////////////////////////////////////////////////////////////////////
 if (isset($_GET['create_view'])) {
   if(! checkAccess(GangliaAcl::ALL_VIEWS, GangliaAcl::EDIT, $conf)) {
@@ -17,23 +31,24 @@ if (isset($_GET['create_view'])) {
     $view_exists = 0;
 
     $available_views = get_available_views();
 
     foreach ($available_views as $view_id => $view) {
-      if ($view['view_name'] == $_GET['view_name']) {
+      if ($view['view_name'] == $user['view_name']) {
         $view_exists = 1;
+        break;
       }
     }
 
     if ($view_exists == 1) {
       $output = "<strong>Alert:</strong> View with the name " .
-                $_GET['view_name'] . 
+                $user['view_name'] . 
                 " already exists.";
     } else {
-      $empty_view = array ("view_name" => $_GET['view_name'],
+      $empty_view = array ("view_name" => $user['view_name'],
                            "items" => array());
-      $view_suffix = str_replace(" ", "_", $_GET['view_name']);
+      $view_suffix = str_replace(" ", "_", $user['view_name']);
       $view_filename = $conf['views_dir'] . "/view_" . preg_replace('/[^a-zA-Z0-9_-]/', '', $view_suffix) . ".json";
       if ( pathinfo( $view_filename, PATHINFO_DIRNAME ) != $conf['views_dir'] ) {
         die('Invalid path detected');
       }
       $json = json_encode($empty_view);
@@ -69,21 +84,22 @@ if (isset($_GET['delete_view'])) {
     $view_exists = 0;
 
     $available_views = get_available_views();
 
     foreach ($available_views as $view_id => $view) {
-      if ($view['view_name'] == $_GET['view_name']) {
+      if ($view['view_name'] == $user['view_name']) {
         $view_exists = 1;
+        break;
       }
     }
 
     if ($view_exists != 1) {
       $output = "<strong>Alert:</strong> View with the name " .
-      $_GET['view_name'] . 
+      $user['view_name'] . 
       " does not exist.";
     } else {
-      $view_suffix = str_replace(" ", "_", $_GET['view_name']);
+      $view_suffix = str_replace(" ", "_", $user['view_name']);
       $view_filename = $conf['views_dir'] . "/view_" . preg_replace('/[^a-zA-Z0-9_-]/', '', $view_suffix) . ".json";
       if ( pathinfo( $view_filename, PATHINFO_DIRNAME ) != $conf['views_dir'] ) {
         die('Invalid path detected');
       }
       if (unlink($view_filename) === FALSE) {
@@ -107,19 +123,19 @@ if (isset($_GET['add_to_view'])) {
     $view_exists = 0;
     // Check whether the view name already exists
     $available_views = get_available_views();
 
     foreach ($available_views as $view_id => $view) {
-      if ($view['view_name'] == $_GET['view_name']) {
+      if ($view['view_name'] == $user['view_name']) {
         $view_exists = 1;
         break;
       }
     }
 
     if ($view_exists == 0) {
       $output = "<strong>Alert:</strong> View " .
-      $_GET['view_name'] . 
+      $user['view_name'] . 
       " does not exist. This should not happen.";
     } else {
       // Read in contents of an existing view
       $view_filename = $view['file_name'];
       // Delete the file_name index

Vulnerable code: $user['viewname'] = isset($_GET["vn"]) ?
    escapeshellcmd( clean_string( rawurldecode($_GET["vn"]) ) ) : '';
<?php

include_once("./eval_conf.php");
include_once("./functions.php");

if( ! checkAccess(GangliaAcl::ALL_VIEWS, GangliaAcl::VIEW, $conf) ) {
  die("You do not have access to view views.");
}

// Load the metric caching code we use if we need to display graphs
retrieve_metrics_cache();

$base = isset($_GET['base']) ? rawurlencode($_GET['base']) . "/" : "";
?>

<html>
<head>
<script type="text/javascript" src="<?php print $base; ?>js/jquery-1.9.1.min.js"></script>
<script type="text/javascript" src="<?php print $base; ?>js/jquery-ui-1.9.1.custom.min.js"></script>
<link type="text/css" href="css/smoothness/jquery-ui-1.9.1.custom.min.css" rel="stylesheet"/>
<link rel="stylesheet" href="./styles.css" type="text/css">
</head>
<body>
  <div>

<?php
  $available_views = get_available_views();
  $view_name = $_GET['vn'];

  // Let's find the view definition
  foreach ($available_views as $view_id => $view) {
    if ($view['view_name'] == $view_name) {
      $view_elements = get_view_graph_elements($view);
      $range_args = "";
      if (isset($_GET['r']) && $_GET['r'] != "") 
	$range_args .= "&amp;r=" . rawurlencode($_GET['r']);
      if (isset($_GET['cs']) && isset($_GET['ce'])) 
	$range_args .= "&amp;cs=" . rawurlencode($_GET['cs']) . "&amp;ce=" . rawurlencode($_GET['ce']);

      if (count($view_elements) != 0) {
	foreach ($view_elements as $id => $element) {
	  $legend = isset($element['hostname']) ? $element['hostname'] : "Aggregate graph";
          $base = isset($_GET['base']) ? rawurlencode($_GET['base']) : '.';
	  print "<a href=\"" . $base . "/graph_all_periods.php?" . htmlentities($element['graph_args']) ."&amp;z=large\"><img title=\"" . $legend . " - " . $element['name'] . "\" border=0 src=\"" . $base . "/graph.php?" . htmlentities($element['graph_args']) . "&amp;z=small" . $range_args .  "\" style=\"padding:2px;\"></a>";
	}
      } else {
	print "No graphs defined for this view. Please add some";
      }
    }
  } 
?>
  </div>
</body>
</html>

      if ($view['view_name'] == $_GET['view_name']) {
                $_GET['view_name'] . 
      $empty_view = array ("view_name" => $_GET['view_name'],
      $view_suffix = str_replace(" ", "_", $_GET['view_name']);
      if ($view['view_name'] == $_GET['view_name']) {
      $_GET['view_name'] . 
      $view_suffix = str_replace(" ", "_", $_GET['view_name']);
      if ($view['view_name'] == $_GET['view_name']) {
      $_GET['view_name'] . 
Fixed code: #------------------------------------------------------------------------------
# Allowed view name characters are alphanumeric plus space, dash and underscore
function is_proper_view_name( $string )
{
  if(preg_match("/[^a-zA-z0-9_\-\ ]/", $string)){
    return false;
  } else {
    return true;
  }
}


$user['viewname'] = '';
if ( isset($_GET["vn"]) &&  is_proper_view_name ($_GET["vn"]) ) {
    $user['viewname'] = $_GET["vn"];
}
$user['view_name'] = $_REQUEST['view_name'];

if( !is_proper_view_name ( $user['view_name'])){
?>
<div class="ui-widget">
  <div class="ui-state-default ui-corner-all" styledefault="padding: 0 .7em;"> 
    <p><span class="ui-icon ui-icon-alert" style="float: left; margin-right: .3em;"></span> 
    View names valid characters are 0-9, a-z, A-Z, -, _ and space. View has not been created.</p>
  </div>
</div>
<?php
  exit(0);
}

      if ($view['view_name'] == $user['view_name']) {
        break;
                $user['view_name'] . 
      $empty_view = array ("view_name" => $user['view_name'],
      $view_suffix = str_replace(" ", "_", $user['view_name']);
      if ($view['view_name'] == $user['view_name']) {
        break;
      $user['view_name'] . 
      $view_suffix = str_replace(" ", "_", $user['view_name']);
      if ($view['view_name'] == $user['view_name']) {
      $user['view_name'] .