Bug 268146: mod_security complain: Invalid cookie format: Cookie value is missing...

diff --git a/Bugzilla/Auth/CGI.pm b/Bugzilla/Auth/CGI.pm
index 2146ca398653b808361ef335496658f3a284dfe7..b374aef796741302480ae6e6b031b222f91c0a38 100644 (file)
--- a/Bugzilla/Auth/CGI.pm
+++ b/Bugzilla/Auth/CGI.pm
@@ -217,12 +217,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 e81cf5da8ce29007558f9cfdc71fbeeafb6d1c6e..e1bfe0cf9a57ff328e67b646435eafa3df654426 100644 (file)
--- a/Bugzilla/CGI.pm
+++ b/Bugzilla/CGI.pm
@@ -19,6 +19,7 @@
 #
 # Contributor(s): Bradley Baetz <bbaetz@student.usyd.edu.au>
 #                 Byron Jones <bugzilla@glob.com.au>
+#                 Marc Schumann <wurblzap@gmail.com>
 
 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;
 
@@ -172,19 +174,44 @@ sub multipart_start {
 sub send_cookie {
     my $self = shift;
 
-    # Add the default path in
-    unshift(@_, '-path' => Param('cookiepath'));
+    # 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;
+    }
+
+    # Complain if -value is not given or empty (bug 268146).
+    if (!exists($paramhash{'-value'}) || !$paramhash{'-value'}) {
+        ThrowCodeError('cookies_need_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;
+    # Add the default path and the domain in.
+    $paramhash{'-path'} = Param('cookiepath');
+    $paramhash{'-domain'} = Param('cookiedomain') if Param('cookiedomain');
 
-    return;
+    # 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));
 }
 
+# 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');
+
+}
 
 1;
 
@@ -232,11 +259,21 @@ Values in C<@exclude> are not included in the result.
 
 =item C<send_cookie>
 
-This routine is identical to CGI.pm's C<cookie> routine, except that the cookie
-is sent to the browser, rather than returned. This should be used by all
-Bugzilla code (instead of C<cookie> or the C<-cookie> argument to C<header>),
-so that under mod_perl the headers can be sent correctly, using C<print> or
-the mod_perl APIs as appropriate.
+This routine is identical to the cookie generation part of CGI.pm's C<cookie>
+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<cookie> or the C<-cookie>
+argument to C<header>), so that under mod_perl the headers can be sent
+correctly, using C<print> or the mod_perl APIs as appropriate.
+
+To remove (expire) a cookie, use C<remove_cookie>.
+
+=item C<remove_cookie>
+
+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.
 
 =back
 

diff --git a/buglist.cgi b/buglist.cgi
index 8653bd7a4f27da51d9177a25b97865a8090f00ae..8654dd0be158553b1825aa80aa1882a39cf8a89d 100755 (executable)
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -654,8 +654,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 {
@@ -961,8 +960,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 261f30a7212d644f142f7315b4eeac1af19462cf..866c3d1855beca6f462aa770ffdedfe120288d73 100755 (executable)
--- a/colchange.cgi
+++ b/colchange.cgi
@@ -91,7 +91,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);
@@ -100,9 +100,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 4a305c1905a4da77a80b2c5a52b2f8a4d052094f..218d69ec1795b2e8abe49d2985479d0e14fca43c 100755 (executable)
--- a/query.cgi
+++ b/query.cgi
@@ -96,8 +96,7 @@ if ($user) {
                 }
                 SendSQL("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 b6cf2f8de94f7107725a2fcb72cb924f247843c0..5053f0b2fe6ac79ac6ca966b533f2c11b720ed5b 100644 (file)
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -82,6 +82,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 <code>[% field FILTER html %]</code>
     and <code>[% type FILTER html %]</code> together.