diff options
author | lpsolit%gmail.com <> | 2009-02-02 18:50:20 +0000 |
---|---|---|
committer | lpsolit%gmail.com <> | 2009-02-02 18:50:20 +0000 |
commit | 2132845310b30bc555ea7ecf5f8d1d4b7a05594c (patch) | |
tree | dca952ceb203ef33aa6553f2bab73115cb7a395c | |
parent | Bug 26257: [SECURITY] Bugzilla should prevent malicious webpages from making ... (diff) | |
download | bugzilla-2132845310b30bc555ea7ecf5f8d1d4b7a05594c.tar.gz bugzilla-2132845310b30bc555ea7ecf5f8d1d4b7a05594c.tar.bz2 bugzilla-2132845310b30bc555ea7ecf5f8d1d4b7a05594c.zip |
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
-rwxr-xr-x | buglist.cgi | 19 | ||||
-rw-r--r-- | template/en/default/account/prefs/saved-searches.html.tmpl | 3 | ||||
-rw-r--r-- | template/en/default/global/user-error.html.tmpl | 5 | ||||
-rw-r--r-- | template/en/default/list/list.html.tmpl | 5 |
4 files changed, 22 insertions, 10 deletions
diff --git a/buglist.cgi b/buglist.cgi index 4e8782f44..3d9228fd0 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -277,7 +277,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 @@ -435,14 +435,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; @@ -491,6 +493,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); @@ -544,9 +550,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; } diff --git a/template/en/default/account/prefs/saved-searches.html.tmpl b/template/en/default/account/prefs/saved-searches.html.tmpl index 241b12fa2..cf458e802 100644 --- a/template/en/default/account/prefs/saved-searches.html.tmpl +++ b/template/en/default/account/prefs/saved-searches.html.tmpl @@ -107,7 +107,8 @@ Remove from <a href="editwhines.cgi">whining</a> first [% ELSE %] <a href="buglist.cgi?cmdtype=dorem&remaction=forget&namedcmd= - [% q.name FILTER url_quote %]">Forget</a> + [% q.name FILTER url_quote %]&token= + [% issue_hash_token([q.id, q.name]) FILTER url_quote %]">Forget</a> [% END %] </td> <td align="center"> diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 565ec1b07..bbae9b39c 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -1349,8 +1349,9 @@ The name <em>[% name FILTER html %]</em> is already used by another saved search. You first have to <a href="buglist.cgi?cmdtype=dorem&remaction=forget&namedcmd= - [%- name FILTER url_quote %]">delete</a> it if you really want to use - this name. + [%- name FILTER url_quote %]&token= + [% issue_hash_token([query_id, name]) FILTER url_quote %]">delete</a> + it if you really want to use this name. [% ELSIF error == "query_name_missing" %] [% title = "No Search Name Specified" %] diff --git a/template/en/default/list/list.html.tmpl b/template/en/default/list/list.html.tmpl index a0c233803..be1741480 100644 --- a/template/en/default/list/list.html.tmpl +++ b/template/en/default/list/list.html.tmpl @@ -214,8 +214,9 @@ <td valign="middle" nowrap="nowrap" class="bz_query_forget"> | <a href="buglist.cgi?cmdtype=dorem&remaction=forget&namedcmd= - [% searchname FILTER url_quote %]">Forget Search ' - [% searchname FILTER html %]'</a> + [% searchname FILTER url_quote %]&token= + [% issue_hash_token([search_id, searchname]) FILTER url_quote %]"> + Forget Search '[% searchname FILTER html %]'</a> </td> [% ELSE %] <td> </td> |