From 430a93b7c4df3aadfa2262ef9b8bcc8e77f0eca8 Mon Sep 17 00:00:00 2001 From: Dave Miller Date: Thu, 29 Aug 2024 07:03:41 -0400 Subject: Bug 1439260: XSS in chart.cgi and report.cgi --- Bugzilla/Chart.pm | 6 ++---- chart.cgi | 22 ++++++++++++++++++++-- report.cgi | 15 +++++++++------ template/en/default/reports/chart.html.tmpl | 7 +++++++ template/en/default/reports/create-chart.html.tmpl | 7 +++++++ template/en/default/reports/report.html.tmpl | 5 +++++ 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 "
Bugzilla::Chart object:";
-    print html_quote(Data::Dumper::Dumper($self));
-    print "
"; + 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 "
data hash:";
-    say html_quote(Data::Dumper::Dumper(%data));
-    say "\ndata array:";
-    say html_quote(Data::Dumper::Dumper(@image_data)) . "\n\n
"; + $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 %] +

Bugzilla::Chart object:

+
+  [% debug_dump FILTER html %]
+  
+[% END %] +
[% 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 %] +

Bugzilla::Chart object:

+
+  [% debug_dump FILTER html %]
+  
+[% 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 %] +

Data hash:

+
[% debug_hash FILTER html %]
+

Data array:

+
[% debug_array FILTER html %]
+

Queries:

[% FOREACH query = queries %]

[% query.sql FILTER html %]

[% END %] -- cgit v1.2.1