Bug 1439260: XSS in chart.cgi and report.cgi

diff --git a/Bugzilla/Chart.pm b/Bugzilla/Chart.pm
index e343a05358d207a490daa887de60629841dba4c1..968d9a09bc8f6c94ad7f6e40caeee1e2f418a551 100644 (file)
--- a/Bugzilla/Chart.pm
+++ b/Bugzilla/Chart.pm
@@ -418,11 +418,9 @@ sub dump {
 
     # Make sure we've read in our data
     my $data = $self->data;
-    
+
     require Data::Dumper;
-    say "<pre>Bugzilla::Chart object:";
-    print html_quote(Data::Dumper::Dumper($self));
-    print "</pre>";
+    return Data::Dumper::Dumper($self);
 }
 
 1;

diff --git a/chart.cgi b/chart.cgi
index 7f21fd098821494ed29f0c68420441404796233a..6e995a4e09535325ed8133715d4ad031cc2b79d0 100755 (executable)
--- a/chart.cgi
+++ b/chart.cgi
@@ -94,6 +94,13 @@ $user->in_group(Bugzilla->params->{"chartgroup"})
 # Only admins may create public queries
 $user->in_group('admin') || $cgi->delete('public');
 
+if ($cgi->param('debug')
+    && Bugzilla->params->{debug_group}
+    && Bugzilla->user->in_group(Bugzilla->params->{debug_group})
+    ) {
+    $vars->{'debug'} = 1;
+}
+
 # All these actions relate to chart construction.
 if ($action =~ /^(assemble|add|remove|sum|subscribe|unsubscribe)$/) {
     # These two need to be done before the creation of the Chart object, so
@@ -304,9 +311,12 @@ sub plot {
     my $format = $template->get_format("reports/chart", "", scalar($cgi->param('ctype')));
 
     # Debugging PNGs is a pain; we need to be able to see the error messages
-    if ($cgi->param('debug')) {
-        print $cgi->header();
-        $vars->{'chart'}->dump();
+    if (exists $vars->{'debug'}) {
+        # Bug 1439260 - if we're using debug mode, always use the HTML template
+        # which has proper filters in it. Debug forces an HTML content type
+        # anyway, and can cause XSS if we're not filtering the output.
+        $format = $template->get_format("reports/chart", "", "html");
+        $vars->{'debug_dump'} = $vars->{'chart'}->dump();
     }
 
     print $cgi->header($format->{'ctype'});
@@ -348,7 +358,9 @@ sub view {
 
     # If we have having problems with bad data, we can set debug=1 to dump
     # the data structure.
-    $chart->dump() if $cgi->param('debug');
+    if (exists $vars->{'debug'}) {
+        $vars->{'debug_dump'} = $chart->dump();
+    }
 
     $template->process("reports/create-chart.html.tmpl", $vars)
       || ThrowTemplateError($template->error());

diff --git a/report.cgi b/report.cgi
index f4f015b92701c661f33e420307c6f7993466a70f..4b1356163c127993bc5e55ce8f314c3faaa2efa2 100755 (executable)
--- a/report.cgi
+++ b/report.cgi
@@ -311,7 +311,12 @@ my $format = $template->get_format("reports/report", $formatparam,
 # If we get a template or CGI error, it comes out as HTML, which isn't valid
 # PNG data, and the browser just displays a "corrupt PNG" message. So, you can
 # set debug=1 to always get an HTML content-type, and view the error.
-$format->{'ctype'} = "text/html" if $cgi->param('debug');
+if (exists $vars->{'debug'}) {
+    # Bug 1439260 - if we're using debug mode, always use the HTML template
+    # which has proper filters in it. Debug forces an HTML content type
+    # anyway, and can cause XSS if we're not filtering the output.
+    $format = $template->get_format("reports/report", $formatparam, "html");
+}
 
 my @time = localtime(time());
 my $date = sprintf "%04d-%02d-%02d", 1900+$time[5],$time[4]+1,$time[3];
@@ -321,12 +326,10 @@ print $cgi->header(-type => $format->{'ctype'},
 
 # Problems with this CGI are often due to malformed data. Setting debug=1
 # prints out both data structures.
-if ($cgi->param('debug')) {
+if (exists $vars->{'debug'}) {
     require Data::Dumper;
-    say "<pre>data hash:";
-    say html_quote(Data::Dumper::Dumper(%data));
-    say "\ndata array:";
-    say html_quote(Data::Dumper::Dumper(@image_data)) . "\n\n</pre>";
+    $vars->{'debug_hash'} = Data::Dumper::Dumper(%data);
+    $vars->{'debug_array'} = Data::Dumper::Dumper(@image_data);
 }
 
 # All formats point to the same section of the documentation.

diff --git a/template/en/default/reports/chart.html.tmpl b/template/en/default/reports/chart.html.tmpl
index ab334639cd83ffd2f5aa5c6da4f43a91b04bb1c0..1e908d95669f952018ae3d1c0e96620df26c5091 100644 (file)
--- a/template/en/default/reports/chart.html.tmpl
+++ b/template/en/default/reports/chart.html.tmpl
@@ -20,6 +20,12 @@
   header_addl_info = time
 %]
 
+[% IF debug %]
+  <p>Bugzilla::Chart object:</p>
+  <pre>
+  [% debug_dump FILTER html %]
+  </pre>
+[% END %]
 <div align="center">
 
   [% imageurl = BLOCK %]chart.cgi?

diff --git a/template/en/default/reports/create-chart.html.tmpl b/template/en/default/reports/create-chart.html.tmpl
index 471a9cb55c7600b5e369091711fa8e3bd677beb0..543d8bd33879c7fb97be02ea3a01d3ec383ffc0e 100644 (file)
--- a/template/en/default/reports/create-chart.html.tmpl
+++ b/template/en/default/reports/create-chart.html.tmpl
@@ -17,6 +17,13 @@
   title = "Create Chart"
 %]
 
+[% IF debug %]
+  <p>Bugzilla::Chart object:</p>
+  <pre>
+  [% debug_dump FILTER html %]
+  </pre>
+[% END %]
+
 [% PROCESS "reports/series-common.html.tmpl" 
   donames = 1 
 %]

diff --git a/template/en/default/reports/report.html.tmpl b/template/en/default/reports/report.html.tmpl
index 2ca5dd90f508e5d3d0b8a80a64fc039e7fd86554..4825e0a66d421634b53f5a67abf08d6e4ad5a959 100644 (file)
--- a/template/en/default/reports/report.html.tmpl
+++ b/template/en/default/reports/report.html.tmpl
@@ -61,6 +61,11 @@
 %]
 
 [% IF debug %]
+  <p>Data hash:</p>
+  <pre>[% debug_hash FILTER html %]</pre>
+  <p>Data array:</p>
+  <pre>[% debug_array FILTER html %]</pre>
+  <p>Queries:</p>
   [% FOREACH query = queries %]
     <p>[% query.sql FILTER html %]</p>
   [% END %]