------------------------------------------------------------------------ r7439 | rony | 2014-03-30 17:52:10 -0500 (Sun, 30 Mar 2014) | 5 lines bug#0002405: SQL injection in graph_xport.php - Fixed form input validation problems - Fixed rrd export and graph shell escape issues ------------------------------------------------------------------------ Index: branches/0.8.8/graph_xport.php =================================================================== --- branches/0.8.8/graph_xport.php (revision 7438) +++ branches/0.8.8/graph_xport.php (revision 7439) @@ -47,43 +47,48 @@ $graph_data_array = array(); +/* ================= input validation ================= */ +input_validate_input_number(get_request_var("local_graph_id")); +input_validate_input_number(get_request_var("rra_id")); +/* ==================================================== */ + /* override: graph start time (unix time) */ -if (!empty($_GET["graph_start"]) && $_GET["graph_start"] < 1600000000) { - $graph_data_array["graph_start"] = $_GET["graph_start"]; +if (!empty($_GET["graph_start"]) && is_numeric($_GET["graph_start"] && $_GET["graph_start"] < 1600000000)) { + $graph_data_array["graph_start"] = get_request_var("graph_start"); } /* override: graph end time (unix time) */ -if (!empty($_GET["graph_end"]) && $_GET["graph_end"] < 1600000000) { - $graph_data_array["graph_end"] = $_GET["graph_end"]; +if (!empty($_GET["graph_end"]) && is_numeric($_GET["graph_end"]) && $_GET["graph_end"] < 1600000000) { + $graph_data_array["graph_end"] = get_request_var("graph_end"); } /* override: graph height (in pixels) */ -if (!empty($_GET["graph_height"]) && $_GET["graph_height"] < 3000) { - $graph_data_array["graph_height"] = $_GET["graph_height"]; +if (!empty($_GET["graph_height"]) && is_numeric($_GET["graph_height"]) && $_GET["graph_height"] < 3000) { + $graph_data_array["graph_height"] = get_request_var("graph_height"); } /* override: graph width (in pixels) */ -if (!empty($_GET["graph_width"]) && $_GET["graph_width"] < 3000) { - $graph_data_array["graph_width"] = $_GET["graph_width"]; +if (!empty($_GET["graph_width"]) && is_numeric($_GET["graph_width"]) && $_GET["graph_width"] < 3000) { + $graph_data_array["graph_width"] = get_request_var("graph_width"); } /* override: skip drawing the legend? */ if (!empty($_GET["graph_nolegend"])) { - $graph_data_array["graph_nolegend"] = $_GET["graph_nolegend"]; + $graph_data_array["graph_nolegend"] = get_request_var("graph_nolegend"); } /* print RRDTool graph source? */ if (!empty($_GET["show_source"])) { - $graph_data_array["print_source"] = $_GET["show_source"]; + $graph_data_array["print_source"] = get_request_var("show_source"); } -$graph_info = db_fetch_row("SELECT * FROM graph_templates_graph WHERE local_graph_id='" . $_REQUEST["local_graph_id"] . "'"); +$graph_info = db_fetch_row("SELECT * FROM graph_templates_graph WHERE local_graph_id='" . get_request_var("local_graph_id") . "'"); /* for bandwidth, NThPercentile */ $xport_meta = array(); /* Get graph export */ -$xport_array = @rrdtool_function_xport($_GET["local_graph_id"], $_GET["rra_id"], $graph_data_array, $xport_meta); +$xport_array = @rrdtool_function_xport($_GET["local_graph_id"], get_request_var("rra_id"), $graph_data_array, $xport_meta); /* Make graph title the suggested file name */ if (is_array($xport_array["meta"])) { Index: branches/0.8.8/lib/rrd.php =================================================================== --- branches/0.8.8/lib/rrd.php (revision 7438) +++ branches/0.8.8/lib/rrd.php (revision 7439) @@ -865,13 +865,13 @@ /* basic graph options */ $graph_opts .= "--imgformat=" . $image_types{$graph["image_format_id"]} . RRD_NL . - "--start=$graph_start" . RRD_NL . - "--end=$graph_end" . RRD_NL . + "--start=" . cacti_escapeshellarg($graph_start) . RRD_NL . + "--end=" . cacti_escapeshellarg($graph_end) . RRD_NL . "--title=" . cacti_escapeshellarg($graph["title_cache"]) . RRD_NL . "$rigid" . - "--base=" . $graph["base_value"] . RRD_NL . - "--height=$graph_height" . RRD_NL . - "--width=$graph_width" . RRD_NL . + "--base=" . cacti_escapeshellarg($graph["base_value"]) . RRD_NL . + "--height=" . cacti_escapeshellarg($graph_height) . RRD_NL . + "--width=" . cacti_escapeshellarg($graph_width) . RRD_NL . "$scale" . "$unit_value" . "$unit_exponent_value" . @@ -1606,8 +1606,8 @@ /* basic export options */ $xport_opts = - "--start=$xport_start" . RRD_NL . - "--end=$xport_end" . RRD_NL . + "--start=" . cacti_escapeshellarg($xport_start) . RRD_NL . + "--end=" . cacti_escapeshellarg($xport_end) . RRD_NL . "--maxrows=10000" . RRD_NL; $xport_defs = ""; @@ -1997,7 +1997,7 @@ $stacked_columns["col" . $j] = ($graph_item_types{$xport_item["graph_type_id"]} == "STACK") ? 1 : 0; $j++; - $txt_xport_items .= "XPORT:" . $data_source_name . ":" . str_replace(":", "", cacti_escapeshellarg($legend_name)) ; + $txt_xport_items .= "XPORT:" . cacti_escapeshellarg($data_source_name) . ":" . str_replace(":", "", cacti_escapeshellarg($legend_name)) ; }else{ $need_rrd_nl = FALSE; }