From f3f3e0055b5c02c1ffb547f816d12b8e4c2a7436 Mon Sep 17 00:00:00 2001 From: "justdave%syndicomm.com" <> Date: Thu, 10 May 2001 10:02:52 +0000 Subject: Fix for bug 38854: reports.cgi needs to escape (untrusted) url params Patch by Myk Melez r= jake@acutex.net --- reports.cgi | 108 ++++++++++++++++++++++++++---------------------------------- 1 file changed, 46 insertions(+), 62 deletions(-) (limited to 'reports.cgi') diff --git a/reports.cgi b/reports.cgi index 8fefd50ac..163f583c8 100755 --- a/reports.cgi +++ b/reports.cgi @@ -35,6 +35,8 @@ # daily stats file, so now works independently of collectstats.pl # version # Added image caching by date and datasets +# Myk Melez \&most_doomed, @@ -72,19 +73,6 @@ my %reports = ConnectToDatabase(1); quietly_check_login(); -print "Content-type: text/html\n"; - -# Changing attachment to inline to resolve 46897 - zach@zachlipton.com -print "Content-disposition: inline; filename=bugzilla_report.html\n\n"; - -# If we're here for the first time, give a banner. Else respect the banner flag. -if ( (!defined $FORM{'product'}) || ($FORM{'banner'}) ) { - PutHeader ("Bug Reports") -} -else { - print("Bug Reports"); -} - GetVersionTable(); # If the usebuggroups parameter is set, we don't want to list all products. @@ -103,54 +91,60 @@ if(Param("usebuggroups")) { push( @myproducts, "-All-", @legal_product ); } -$FORM{'output'} ||= "most_doomed"; # a reasonable default - if (! defined $FORM{'product'}) { + + print "Content-type: text/html\n\n"; + PutHeader("Bug Reports"); &choose_product; -} -else { + PutFooter(); + +} else { + + # For security and correctness, validate the value of the "product" form variable. + # Valid values are those products for which the user has permissions which appear + # in the "product" drop-down menu on the report generation form. + grep($_ eq $FORM{'product'}, @myproducts) + || DisplayError("You entered an invalid product name.") && exit; + # If usebuggroups is on, we don't want people to be able to view # reports for products they don't have permissions for... - if(Param("usebuggroups") && - GroupExists($FORM{'product'}) && - !UserInGroup($FORM{'product'})) - { - print "

Permission denied.

\n"; - print "Sorry; you do not have the permissions necessary to view\n"; - print "reports for this product.\n"; - print "

\n"; - PutFooter(); - exit; - } + Param("usebuggroups") + && GroupExists($FORM{'product'}) + && !UserInGroup($FORM{'product'}) + && DisplayError("You do not have the permissions necessary to view reports for this product.") + && exit; - # we want to be careful about what subroutines - # can be called from outside. modify %reports - # accordingly when a new report type is added - - if (! exists $reports{$FORM{'output'}}) { - $FORM{'output'} = "most_doomed"; # a reasonable default + # For security and correctness, validate the value of the "output" form variable. + # Valid values are the keys from the %reports hash defined above which appear in + # the "output" drop-down menu on the report generation form. + $FORM{'output'} ||= "most_doomed"; # a reasonable default + grep($_ eq $FORM{'output'}, keys %reports) + || DisplayError("You entered an invalid output type.") + && exit; + + # Output appropriate HTTP response headers + print "Content-type: text/html\n"; + # Changing attachment to inline to resolve 46897 - zach@zachlipton.com + print "Content-disposition: inline; filename=bugzilla_report.html\n\n"; + + if ($FORM{'banner'}) { + PutHeader("Bug Reports"); + } + else { + print("Bug Reports"); } - - my $f = $reports{$FORM{'output'}}; - if (! defined $f) { - print "start over, your form data was all messed up.

\n"; - foreach (keys %::FORM) { - print "$_ : " . - ($FORM{$_} ? $FORM{$_} : "undef") . "
\n"; - } - PutFooter() if $FORM{banner}; - exit; - } + # Execute the appropriate report generation function + # (the one whose name is the same as the value of the "output" form variable). + &{$reports{$FORM{'output'}}}; - &{$f}; -} + # ??? why is this necessary? formatting looks fine without it + print "

"; -print < -FIN + PutFooter() if $FORM{banner}; + +} -PutFooter() if $FORM{banner}; ################################## @@ -257,7 +251,6 @@ FIN FIN #Add this above to get a control for showing the SQL query: # Show SQL
- PutFooter(); } sub most_doomed { @@ -485,11 +478,6 @@ FIN FIN } -sub is_legal_product { - my $product = shift; - return grep { $_ eq $product} @myproducts; -} - sub daily_stats_filename { my ($prodname) = @_; $prodname =~ s/\//-/gs; @@ -501,10 +489,6 @@ sub show_chart { # here. Should probably return some decent error message. return unless $use_gd; - if (! is_legal_product ($FORM{'product'})) { - &die_politely ("Unknown product: $FORM{'product'}"); - } - if (! $FORM{datasets}) { die_politely("You didn't select any datasets to plot"); } -- cgit v1.2.3-24-g4f1b