From f1f75e76d7f23b1ae3d324422819be53dee8742d Mon Sep 17 00:00:00 2001
From: "lpsolit%gmail.com" <>
Date: Thu, 6 Nov 2008 00:58:04 +0000
Subject: [PATCH] Bug 449931: [SECURITY] Unprivileged users can
 approve/unapprove all the quips (including bypassing moderation) - Patch by
 Robin H. Johnson <robbat2@gentoo.org> r/a=LpSolit

---
 quips.cgi                                     | 22 ++++++++++++++-----
 .../en/default/global/user-error.html.tmpl    |  2 ++
 template/en/default/list/quips.html.tmpl      |  7 ++++--
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/quips.cgi b/quips.cgi
index d811ee5fe4..398bae4c5a 100755
--- a/quips.cgi
+++ b/quips.cgi
@@ -87,6 +87,11 @@ if ($action eq "add") {
 }
 
 if ($action eq 'approve') {
+    Bugzilla->user->in_group('admin')
+      || ThrowUserError("auth_failure", {group  => "admin",
+                                         action => "approve",
+                                         object => "quips"});
+ 
     # Read in the entire quip list
     SendSQL("SELECT quipid, approved FROM quips");
  
@@ -99,11 +104,18 @@ if ($action eq 'approve') {
     my @approved;
     my @unapproved;
     foreach my $quipid (keys %quips) {
-       my $form = $cgi->param('quipid_'.$quipid) ? 1 : 0;
-       if($quips{$quipid} ne $form) {
-           if($form) { push(@approved, $quipid); }
-           else { push(@unapproved, $quipid); }
-       }
+        # Must check for each quipid being defined for concurrency and
+        # automated usage where only one quipid might be defined.
+        my $quip = $cgi->param("quipid_$quipid") ? 1 : 0;
+        if(defined($cgi->param("defined_quipid_$quipid"))) {
+            if($quips{$quipid} != $quip) {
+                if($quip) { 
+                    push(@approved, $quipid); 
+                } else { 
+                    push(@unapproved, $quipid); 
+                }
+            }
+        }
     }
     SendSQL("UPDATE quips SET approved = 1 WHERE quipid IN (" .
             join(",", @approved) . ")") if($#approved > -1);
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index cf7dd2cd53..08c1b3eae2 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -140,6 +140,8 @@
       schedule
     [% ELSIF action == "use" %]
       use
+    [% ELSIF action == "approve" %]
+      approve
     [% END %]
 
     [% IF object == "attachment" %]
diff --git a/template/en/default/list/quips.html.tmpl b/template/en/default/list/quips.html.tmpl
index 92570777ed..fa09171b2b 100644
--- a/template/en/default/list/quips.html.tmpl
+++ b/template/en/default/list/quips.html.tmpl
@@ -125,8 +125,11 @@
               </a>
             </td>
             <td>
-              <input type="checkbox" name="quipid_[% quipid FILTER uri%]"
-                     id="quipid_[% quipid FILTER uri%]"
+              <input type="hidden" name="defined_quipid_[% quipid FILTER html %]"
+                     id="defined_quipid_[% quipid FILTER html %]"
+                     value="1">
+              <input type="checkbox" name="quipid_[% quipid FILTER html %]"
+                     id="quipid_[% quipid FILTER html %]"
                      [%- ' checked="checked"' IF quips.$quipid.approved %]>
             </td>
           </tr>
-- 
2.47.2