Bug 466748: [SECURITY] Shared/saved searches can be deleted without user confirmation using predictable URL - Patch by Frédéric Buclin <LpSolit@gmail.com> r=mkanat a=LpSolit

1 files changed, 14 insertions, 5 deletions

diff --git a/buglist.cgi b/buglist.cgi
index f5284439c..de7e56bd7 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -290,7 +290,7 @@ sub LookupNamedQuery {
     $result
        || ThrowUserError("buglist_parameters_required", {'queryname' => $name});
 
-    return $result;
+    return wantarray ? ($result, $id) : $result;
 }
 
 # Inserts a Named Query (a "Saved Search") into the database, or
@@ -448,14 +448,16 @@ $filename =~ s/"/\\"/g; # escape quotes
 # Take appropriate action based on user's request.
 if ($cgi->param('cmdtype') eq "dorem") {  
     if ($cgi->param('remaction') eq "run") {
-        $buffer = LookupNamedQuery(scalar $cgi->param("namedcmd"),
-                                   scalar $cgi->param('sharer_id'));
+        my $query_id;
+        ($buffer, $query_id) = LookupNamedQuery(scalar $cgi->param("namedcmd"),
+                                                scalar $cgi->param('sharer_id'));
         # If this is the user's own query, remember information about it
         # so that it can be modified easily.
         $vars->{'searchname'} = $cgi->param('namedcmd');
         if (!$cgi->param('sharer_id') ||
             $cgi->param('sharer_id') == Bugzilla->user->id) {
             $vars->{'searchtype'} = "saved";
+            $vars->{'search_id'} = $query_id;
         }
         $params = new Bugzilla::CGI($buffer);
         $order = $params->param('order') || $order;
@@ -504,6 +506,10 @@ if ($cgi->param('cmdtype') eq "dorem") {
             # The user has no query of this name. Play along.
         }
         else {
+            # Make sure the user really wants to delete his saved search.
+            my $token = $cgi->param('token');
+            check_hash_token($token, [$query_id, $qname]);
+
             $dbh->do('DELETE FROM namedqueries
                             WHERE id = ?',
                      undef, $query_id);
@@ -557,9 +563,12 @@ elsif (($cgi->param('cmdtype') eq "doit") && defined $cgi->param('remtype')) {
             my %bug_ids;
             my $is_new_name = 0;
             if ($query_name) {
+                my ($query, $query_id) =
+                  LookupNamedQuery($query_name, undef, QUERY_LIST, !THROW_ERROR);
                 # Make sure this name is not already in use by a normal saved search.
-                if (LookupNamedQuery($query_name, undef, QUERY_LIST, !THROW_ERROR)) {
-                    ThrowUserError('query_name_exists', {'name' => $query_name});
+                if ($query) {
+                    ThrowUserError('query_name_exists', {name     => $query_name,
+                                                         query_id => $query_id});
                 }
                 $is_new_name = 1;
             }