From 095e8f1f039d7f7425ed32515d5964a84dcbf315 Mon Sep 17 00:00:00 2001
From: Dave Miller <justdave@bugzilla.org>
Date: Thu, 29 Aug 2024 07:02:08 -0400
Subject: [PATCH] Bug 1439260: XSS in chart.cgi and report.cgi

---
 Bugzilla/Chart.pm                             |  6 ++----
 chart.cgi                                     | 20 +++++++++++++++----
 report.cgi                                    | 15 ++++++++------
 template/en/default/reports/chart.html.tmpl   |  6 ++++++
 .../en/default/reports/create-chart.html.tmpl |  7 +++++++
 template/en/default/reports/report.html.tmpl  |  5 +++++
 6 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/Bugzilla/Chart.pm b/Bugzilla/Chart.pm
index e343a05358..968d9a09bc 100644
--- 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 7f21fd0988..6e995a4e09 100755
--- 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 f4f015b927..4b1356163c 100755
--- 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 ab334639cd..1e908d9566 100644
--- 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 471a9cb55c..543d8bd338 100644
--- 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 2ca5dd90f5..4825e0a66d 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 %]
-- 
2.47.3