From: lpsolit%gmail.com <>
Date: Mon, 2 Feb 2009 19:04:31 +0000 (+0000)
Subject: Bug 466692: [SECURITY] keywords and unused flag types can be deleted by bypassing... 
X-Git-Tag: bugzilla-2.22.7~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=88a457f0405de456d3e9fae93ba97bdc7a00ae54;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 <LpSolit@gmail.com> r=mkanat a=LpSolit
---

diff --git a/editflagtypes.cgi b/editflagtypes.cgi
index 6ed20c47c3..ac71dba367 100755
--- a/editflagtypes.cgi
+++ b/editflagtypes.cgi
@@ -389,16 +389,13 @@ sub update {
 }
 
 
-sub confirmDelete 
-{
-  my $id = validateID();
-
-  # check if we need confirmation to delete:
-  
-  my $count = Bugzilla::Flag::count({ 'type_id' => $id,
-                                      'is_active' => 1 });
-  
-  if ($count > 0) {
+sub confirmDelete {
+    my $id = validateID();
+
+    # check if we need confirmation to delete:
+    my $count = Bugzilla::Flag::count({ 'type_id' => $id,
+                                        'is_active' => 1 });
+
     $vars->{'flag_type'} = Bugzilla::FlagType::get($id);
     $vars->{'flag_count'} = scalar($count);
     $vars->{'token'} = issue_session_token('delete_flagtype');
@@ -408,11 +405,6 @@ 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 {
-    my $token = issue_session_token('delete_flagtype');
-    deleteType($token);
-  }
 }
 
 
diff --git a/editkeywords.cgi b/editkeywords.cgi
index e008cffc00..04f045322e 100755
--- a/editkeywords.cgi
+++ b/editkeywords.cgi
@@ -247,36 +247,34 @@ if ($action eq 'update') {
 }
 
 
-if ($action eq 'delete') {
+if ($action eq 'del') {
     my $id = ValidateKeyID(scalar $cgi->param('id'));
 
     my $name = $dbh->selectrow_array('SELECT name FROM keyworddefs
                                       WHERE id= ?', undef, $id);
 
-    if (!$cgi->param('reallydelete')) {
-        my $bugs = $dbh->selectrow_array('SELECT COUNT(*) FROM keywords
-                                          WHERE keywordid = ?',
-                                          undef, $id);
-
-        # We need this token even if there is no bug using this keyword.
-        $token = issue_session_token('delete_keyword');
+    my $bugs = $dbh->selectrow_array('SELECT COUNT(*) FROM keywords
+                                      WHERE keywordid = ?',
+                                      undef, $id);
 
-        if ($bugs) {
-            $vars->{'bug_count'} = $bugs;
-            $vars->{'keyword_id'} = $id;
-            $vars->{'name'} = $name;
-            $vars->{'token'} = $token;
+    $vars->{'bug_count'} = $bugs;
+    $vars->{'keyword_id'} = $id;
+    $vars->{'name'} = $name;
+    $vars->{'token'} = issue_session_token('delete_keyword');
 
-            print $cgi->header();
+    print $cgi->header();
 
-            $template->process("admin/keywords/confirm-delete.html.tmpl", $vars)
-              || ThrowTemplateError($template->error());
+    $template->process("admin/keywords/confirm-delete.html.tmpl", $vars)
+      || ThrowTemplateError($template->error());
+    exit;
+}
 
-            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 $id = ValidateKeyID(scalar $cgi->param('id'));
+
+    my $name = $dbh->selectrow_array('SELECT name FROM keyworddefs
+                                      WHERE id= ?', undef, $id);
 
     $dbh->do('DELETE FROM keywords WHERE keywordid = ?', undef, $id);
     $dbh->do('DELETE FROM keyworddefs WHERE id = ?', undef, $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 e67412a1c2..411a8a3933 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 %]
 
 <p>
-   There are [% 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_count %]
+    There are [% 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
    <a href="editflagtypes.cgi?action=deactivate&amp;id=[% flag_type.id %]&amp;token=
            [%- token FILTER html %]">deactivate it</a>,
-   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_count %] and its flags [% END %] will remain
+  in the database but will not appear in the [% terms.Bugzilla %] UI.
 </p>
 
 <table>
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 3329a54732..e079f5d568
--- a/template/en/default/admin/keywords/confirm-delete.html.tmpl
+++ b/template/en/default/admin/keywords/confirm-delete.html.tmpl
@@ -33,7 +33,7 @@
 <p>
   [% IF bug_count == 1 %]
     There is one [% terms.bug %] with this keyword set.
-  [% ELSE %]
+  [% ELSIF bug_count > 1 %]
     There are [% bug_count %] [%+terms.bugs %] with
     this keyword set.
   [% END %]
@@ -45,7 +45,6 @@
 <form method="post" action="editkeywords.cgi">
   <input type="hidden" name="id" value="[% keyword_id %]">
   <input type="hidden" name="action" value="delete">
-  <input type="hidden" name="reallydelete" value="1">
   <input type="hidden" name="token" value="[% token FILTER html %]">
   <input type="submit" value="Yes, really delete the keyword">
 </form>
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 0853332732..41b7903ea8
--- 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&amp;id=%%id%%"
+       contentlink => "editkeywords.cgi?action=del&amp;id=%%id%%"
      }
    ]
 %]