From: lpsolit%gmail.com <> Date: Mon, 2 Feb 2009 19:03:01 +0000 (+0000) Subject: Bug 466692: [SECURITY] keywords and unused flag types can be deleted by bypassing... X-Git-Tag: bugzilla-3.0.7~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0feef0ab69bf5243d7c838f417fc2d70b2fef27a;p=thirdparty%2Fbugzilla.git Bug 466692: [SECURITY] keywords and unused flag types can be deleted by bypassing the token check - Patch by Frédéric Buclin r=mkanat a=LpSolit --- diff --git a/editflagtypes.cgi b/editflagtypes.cgi index 0aec0385d6..3804f6fae2 100755 --- a/editflagtypes.cgi +++ b/editflagtypes.cgi @@ -80,7 +80,7 @@ elsif ($action eq 'edit') { edit($action); } elsif ($action eq 'insert') { insert($token); } elsif ($action eq 'update') { update($token); } elsif ($action eq 'confirmdelete') { confirmDelete(); } -elsif ($action eq 'delete') { deleteType(undef, $token); } +elsif ($action eq 'delete') { deleteType($token); } elsif ($action eq 'deactivate') { deactivate($token); } else { ThrowCodeError("action_unrecognized", { action => $action }); @@ -445,9 +445,8 @@ sub update { sub confirmDelete { - my $flag_type = validateID(); + my $flag_type = validateID(); - if ($flag_type->flag_count) { $vars->{'flag_type'} = $flag_type; $vars->{'token'} = issue_session_token('delete_flagtype'); # Return the appropriate HTTP response headers. @@ -456,20 +455,13 @@ sub confirmDelete { # Generate and return the UI (HTML page) from the appropriate template. $template->process("admin/flag-type/confirm-delete.html.tmpl", $vars) || ThrowTemplateError($template->error()); - } - else { - # We should *always* ask if the admin really wants to delete - # a flagtype, even if there is no flag belonging to this type. - my $token = issue_session_token('delete_flagtype'); - deleteType($flag_type, $token); - } } sub deleteType { - my $flag_type = shift || validateID(); my $token = shift; check_token_data($token, 'delete_flagtype'); + my $flag_type = validateID(); my $id = $flag_type->id; my $dbh = Bugzilla->dbh; diff --git a/editkeywords.cgi b/editkeywords.cgi index 3aca22e438..92bb361156 100755 --- a/editkeywords.cgi +++ b/editkeywords.cgi @@ -145,26 +145,23 @@ if ($action eq 'update') { exit; } - -if ($action eq 'delete') { +if ($action eq 'del') { my $keyword = new Bugzilla::Keyword($key_id) || ThrowCodeError('invalid_keyword_id', { id => $key_id }); $vars->{'keyword'} = $keyword; + $vars->{'token'} = issue_session_token('delete_keyword'); - # We need this token even if there is no bug using this keyword. - $token = issue_session_token('delete_keyword'); - - if (!$cgi->param('reallydelete') && $keyword->bug_count) { - $vars->{'token'} = $token; + print $cgi->header(); + $template->process("admin/keywords/confirm-delete.html.tmpl", $vars) + || ThrowTemplateError($template->error()); + exit; +} - print $cgi->header(); - $template->process("admin/keywords/confirm-delete.html.tmpl", $vars) - || ThrowTemplateError($template->error()); - exit; - } - # We cannot do this check earlier as we have to check 'reallydelete' first. +if ($action eq 'delete') { check_token_data($token, 'delete_keyword'); + my $keyword = new Bugzilla::Keyword($key_id) + || ThrowCodeError('invalid_keyword_id', { id => $key_id }); $dbh->do('DELETE FROM keywords WHERE keywordid = ?', undef, $keyword->id); $dbh->do('DELETE FROM keyworddefs WHERE id = ?', undef, $keyword->id); diff --git a/template/en/default/admin/flag-type/confirm-delete.html.tmpl b/template/en/default/admin/flag-type/confirm-delete.html.tmpl index 0af9fb5a20..94c09855d6 100644 --- a/template/en/default/admin/flag-type/confirm-delete.html.tmpl +++ b/template/en/default/admin/flag-type/confirm-delete.html.tmpl @@ -26,13 +26,16 @@ [% PROCESS global/header.html.tmpl title = title %]

- There are [% flag_type.flag_count %] flags of type [% flag_type.name FILTER html %]. - If you delete this type, those flags will also be deleted. Note that - instead of deleting the type you can + [% IF flag_type.flag_count %] + There are [% flag_type.flag_count %] flags of type [% flag_type.name FILTER html %]. + If you delete this type, those flags will also be deleted. + [% END %] + + Note that instead of deleting the type you can deactivate it, - in which case the type and its flags will remain in the database - but will not appear in the [% terms.Bugzilla %] UI. + in which case the type [% IF flag_type.flag_count %] and its flags [% END %] will remain + in the database but will not appear in the [% terms.Bugzilla %] UI.

diff --git a/template/en/default/admin/keywords/confirm-delete.html.tmpl b/template/en/default/admin/keywords/confirm-delete.html.tmpl old mode 100755 new mode 100644 index 0d68524d78..7b6ebd512e --- a/template/en/default/admin/keywords/confirm-delete.html.tmpl +++ b/template/en/default/admin/keywords/confirm-delete.html.tmpl @@ -32,7 +32,7 @@

[% IF keyword.bug_count == 1 %] There is one [% terms.bug %] with this keyword set. - [% ELSE %] + [% ELSIF keyword.bug_count > 1 %] There are [% keyword.bug_count FILTER html %] [%+ terms.bugs %] with this keyword set. [% END %] @@ -44,7 +44,6 @@
- diff --git a/template/en/default/admin/keywords/list.html.tmpl b/template/en/default/admin/keywords/list.html.tmpl old mode 100755 new mode 100644 index 90b079fc46..f07f962ee7 --- a/template/en/default/admin/keywords/list.html.tmpl +++ b/template/en/default/admin/keywords/list.html.tmpl @@ -55,7 +55,7 @@ { heading => "Action" content => "Delete" - contentlink => "editkeywords.cgi?action=delete&id=%%id%%" + contentlink => "editkeywords.cgi?action=del&id=%%id%%" } ] %]