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

6 files changed, 50 insertions, 12 deletions

diff --git a/Bugzilla/Chart.pm b/Bugzilla/Chart.pm
index 3c69006aa..faf7a4785 100644
--- a/Bugzilla/Chart.pm
+++ b/Bugzilla/Chart.pm
@@ -420,11 +420,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 c1bafa117..18ab87e5e 100755
--- a/chart.cgi
+++ b/chart.cgi
@@ -96,6 +96,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
@@ -312,7 +319,16 @@ sub plot {
     disable_utf8() if ($format->{'ctype'} =~ /^image\//);
 
     # Debugging PNGs is a pain; we need to be able to see the error messages
-    $vars->{'chart'}->dump() 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/chart", "", "html");
+        $vars->{'debug_dump'} = $vars->{'chart'}->dump();
+    }
+
+    print $cgi->header($format->{'ctype'});
+    disable_utf8() if ($format->{'ctype'} =~ /^image\//);
 
     $template->process($format->{'template'}, $vars)
       || ThrowTemplateError($template->error());
@@ -350,7 +366,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 2a8317d7a..d5f471ef0 100755
--- a/report.cgi
+++ b/report.cgi
@@ -359,19 +359,22 @@ 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");
+}
 
 $cgi->set_dated_content_disp("inline", "report", $format->{extension});
 print $cgi->header($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 dfab725e6..7004086de 100644
--- a/template/en/default/reports/chart.html.tmpl
+++ b/template/en/default/reports/chart.html.tmpl
@@ -20,6 +20,13 @@
   header_addl_info = time
 %]
 
+[% IF debug %]
+  <p>Bugzilla::Chart object:</p>
+  <pre>
+  [% debug_dump FILTER html %]
+  </pre>
+[% END %]
+
 <div class="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 6b5fa5fe3..9ae25c608 100644
--- a/template/en/default/reports/create-chart.html.tmpl
+++ b/template/en/default/reports/create-chart.html.tmpl
@@ -18,6 +18,13 @@
   style_urls = ['skins/standard/buglist.css']
 %]
 
+[% 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 a9cd96551..b669070c2 100644
--- 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 %]