From 73270363b7dabda4406b5ab638ead98a951eebeb Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Thu, 7 Jul 2005 18:53:28 +0000 Subject: Bug 268146: mod_security complain: Invalid cookie format: Cookie value is missing #2 - Patch by Marc Schumann r=kiko a=justdave --- Bugzilla/Auth/Login/WWW/CGI.pm | 8 +-- Bugzilla/CGI.pm | 67 ++++++++++++++++++------- buglist.cgi | 6 +-- colchange.cgi | 13 +++-- query.cgi | 3 +- template/en/default/global/code-error.html.tmpl | 3 ++ 6 files changed, 67 insertions(+), 33 deletions(-) diff --git a/Bugzilla/Auth/Login/WWW/CGI.pm b/Bugzilla/Auth/Login/WWW/CGI.pm index 98fd3a6d3..d117aef47 100644 --- a/Bugzilla/Auth/Login/WWW/CGI.pm +++ b/Bugzilla/Auth/Login/WWW/CGI.pm @@ -232,12 +232,8 @@ sub logout { sub clear_browser_cookies { my $cgi = Bugzilla->cgi; - $cgi->send_cookie(-name => "Bugzilla_login", - -value => "", - -expires => "Tue, 15-Sep-1998 21:49:00 GMT"); - $cgi->send_cookie(-name => "Bugzilla_logincookie", - -value => "", - -expires => "Tue, 15-Sep-1998 21:49:00 GMT"); + $cgi->remove_cookie('Bugzilla_login'); + $cgi->remove_cookie('Bugzilla_logincookie'); } 1; diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm index c4433cc62..6f5a6f6d7 100644 --- a/Bugzilla/CGI.pm +++ b/Bugzilla/CGI.pm @@ -19,6 +19,7 @@ # # Contributor(s): Bradley Baetz # Byron Jones +# Marc Schumann use strict; @@ -28,6 +29,7 @@ use CGI qw(-no_xhtml -oldstyle_urls :private_tempfiles :unique_headers SERVER_PU use base qw(CGI); +use Bugzilla::Error; use Bugzilla::Util; use Bugzilla::Config; @@ -177,21 +179,42 @@ sub multipart_start { sub send_cookie { my $self = shift; - # Add the default path in - unshift(@_, '-path' => Param('cookiepath')); - if (Param('cookiedomain')) - { - unshift(@_, '-domain' => Param('cookiedomain')); + # Move the param list into a hash for easier handling. + my %paramhash; + my @paramlist; + my ($key, $value); + while ($key = shift) { + $value = shift; + $paramhash{$key} = $value; } - # Use CGI::Cookie directly, because CGI.pm's |cookie| method gives the - # current value if there isn't a -value attribute, which happens when - # we're expiring an entry. - require CGI::Cookie; - my $cookie = CGI::Cookie->new(@_); - push @{$self->{Bugzilla_cookie_list}}, $cookie; + # Complain if -value is not given or empty (bug 268146). + if (!exists($paramhash{'-value'}) || !$paramhash{'-value'}) { + ThrowCodeError('cookies_need_value'); + } + + # Add the default path and the domain in. + $paramhash{'-path'} = Param('cookiepath'); + $paramhash{'-domain'} = Param('cookiedomain') if Param('cookiedomain'); + + # Move the param list back into an array for the call to cookie(). + foreach (keys(%paramhash)) { + unshift(@paramlist, $_ => $paramhash{$_}); + } + + push(@{$self->{'Bugzilla_cookie_list'}}, $self->cookie(@paramlist)); +} - return; +# Cookies are removed by setting an expiry date in the past. +# This method is a send_cookie wrapper doing exactly this. +sub remove_cookie { + my $self = shift; + my ($cookiename) = (@_); + + # Expire the cookie, giving a non-empty dummy value (bug 268146). + $self->send_cookie('-name' => $cookiename, + '-expires' => 'Tue, 15-Sep-1998 21:49:00 GMT', + '-value' => 'X'); } # Redirect to https if required @@ -256,11 +279,21 @@ Values in C<@exclude> are not included in the result. =item C -This routine is identical to CGI.pm's C routine, except that the cookie -is sent to the browser, rather than returned. This should be used by all -Bugzilla code (instead of C or the C<-cookie> argument to C
), -so that under mod_perl the headers can be sent correctly, using C or -the mod_perl APIs as appropriate. +This routine is identical to the cookie generation part of CGI.pm's C +routine, except that it knows about Bugzilla's cookie_path and cookie_domain +parameters and takes them into account if necessary. +This should be used by all Bugzilla code (instead of C or the C<-cookie> +argument to C
), so that under mod_perl the headers can be sent +correctly, using C or the mod_perl APIs as appropriate. + +To remove (expire) a cookie, use C. + +=item C + +This is a wrapper around send_cookie, setting an expiry date in the past, +effectively removing the cookie. + +As its only argument, it takes the name of the cookie to expire. =item C diff --git a/buglist.cgi b/buglist.cgi index 40f54cb38..edacd836e 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -707,8 +707,7 @@ if ($order) { else { my $vars = { fragment => $fragment }; if ($order_from_cookie) { - $cgi->send_cookie(-name => 'LASTORDER', - -expires => 'Tue, 15-Sep-1998 21:49:00 GMT'); + $cgi->remove_cookie('LASTORDER'); ThrowCodeError("invalid_column_name_cookie", $vars); } else { @@ -1020,8 +1019,7 @@ if ($format->{'extension'} eq "html") { -expires => 'Fri, 01-Jan-2038 00:00:00 GMT'); } else { - $cgi->send_cookie(-name => 'BUGLIST', - -expires => 'Tue, 15-Sep-1998 21:49:00 GMT'); + $cgi->remove_cookie('BUGLIST'); $vars->{'toolong'} = 1; } diff --git a/colchange.cgi b/colchange.cgi index 11caca423..235da6a01 100755 --- a/colchange.cgi +++ b/colchange.cgi @@ -97,7 +97,7 @@ if (defined $cgi->param('rememberedquery')) { } } if (defined $cgi->param('splitheader')) { - $splitheader = $cgi->param('splitheader'); + $splitheader = $cgi->param('splitheader')? 1: 0; } } my $list = join(" ", @collist); @@ -106,9 +106,14 @@ if (defined $cgi->param('rememberedquery')) { $cgi->send_cookie(-name => 'COLUMNLIST', -value => $list, -expires => 'Fri, 01-Jan-2038 00:00:00 GMT'); - $cgi->send_cookie(-name => 'SPLITHEADER', - -value => $cgi->param('splitheader'), - -expires => 'Fri, 01-Jan-2038 00:00:00 GMT'); + if ($splitheader) { + $cgi->send_cookie(-name => 'SPLITHEADER', + -value => $splitheader, + -expires => 'Fri, 01-Jan-2038 00:00:00 GMT'); + } + else { + $cgi->remove_cookie('SPLITHEADER'); + } $vars->{'message'} = "change_columns"; $vars->{'redirect_url'} = "buglist.cgi?".$cgi->param('rememberedquery'); diff --git a/query.cgi b/query.cgi index 316d04de7..fa4a791f8 100755 --- a/query.cgi +++ b/query.cgi @@ -100,8 +100,7 @@ if ($userid) { } $dbh->bz_unlock_tables(); } - $cgi->send_cookie(-name => $cookiename, - -expires => "Fri, 01-Jan-2038 00:00:00 GMT"); + $cgi->remove_cookie($cookiename); } } } diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl index d12036cd7..d54163bd8 100644 --- a/template/en/default/global/code-error.html.tmpl +++ b/template/en/default/global/code-error.html.tmpl @@ -86,6 +86,9 @@ Charts will not work without the Chart::Lines Perl module being installed. Run checksetup.pl for installation instructions. + [% ELSIF error == "cookies_need_value" %] + Every cookie must have a value. + [% ELSIF error == "field_type_mismatch" %] Cannot seem to handle [% field FILTER html %] and [% type FILTER html %] together. -- cgit v1.2.3-24-g4f1b