From d382992164347e076c51d3116a32aeabb2beecd5 Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Mon, 2 Feb 2009 18:59:17 +0000 Subject: 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 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- editflagtypes.cgi | 14 +++---------- editkeywords.cgi | 23 ++++++++++------------ .../admin/flag-type/confirm-delete.html.tmpl | 13 +++++++----- .../admin/keywords/confirm-delete.html.tmpl | 3 +-- template/en/default/admin/keywords/list.html.tmpl | 2 +- 5 files changed, 23 insertions(+), 32 deletions(-) mode change 100755 => 100644 template/en/default/admin/keywords/confirm-delete.html.tmpl mode change 100755 => 100644 template/en/default/admin/keywords/list.html.tmpl diff --git a/editflagtypes.cgi b/editflagtypes.cgi index d77c6b8a3..4dbaae573 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 }); @@ -460,9 +460,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. @@ -471,20 +470,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 dbc92971d..76bba4817 100755 --- a/editkeywords.cgi +++ b/editkeywords.cgi @@ -151,26 +151,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 cc6a064a9..ed909417d 100644 --- a/template/en/default/admin/flag-type/confirm-delete.html.tmpl +++ b/template/en/default/admin/flag-type/confirm-delete.html.tmpl @@ -28,13 +28,16 @@ %]

- 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 6bde05abf..20a6deee7 --- a/template/en/default/admin/keywords/confirm-delete.html.tmpl +++ b/template/en/default/admin/keywords/confirm-delete.html.tmpl @@ -31,7 +31,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 %] @@ -43,7 +43,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 5fb6b3aa6..c400a2362 --- a/template/en/default/admin/keywords/list.html.tmpl +++ b/template/en/default/admin/keywords/list.html.tmpl @@ -54,7 +54,7 @@ { heading => "Action" content => "Delete" - contentlink => "editkeywords.cgi?action=delete&id=%%id%%" + contentlink => "editkeywords.cgi?action=del&id=%%id%%" } ] %] -- cgit v1.2.3-24-g4f1b