Bug 121074 - taint error after changing bug

r=daa@distributed.net,jake

diff --git a/globals.pl b/globals.pl
index 37ecdafbeae2a8319b70f27130cbc3a2eb1a410c..d8e5672e056d674000ab12c716e9d4f79f709d05 100644 (file)
--- a/globals.pl
+++ b/globals.pl
@@ -213,7 +213,7 @@ sub SendSQL {
     # b) We want to know who called SendSQL...
     # Is there a better way to do b?
     if (is_tainted($str)) {
-        die "Attempted to send tainted string to the database";
+        die "Attempted to send tainted string '$str' to the database";
     }
 
     my $iswrite =  ($str =~ /^(INSERT|REPLACE|UPDATE|DELETE)/i);

diff --git a/process_bug.cgi b/process_bug.cgi
index 13942ca5fd9303239698825e9980c8e676fd0f87..dcde93035916e7dbc1c0b0a7cca36e2b83294eeb 100755 (executable)
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -92,20 +92,6 @@ if (defined $::FORM{'dup_id'} && $::FORM{'knob'} eq "duplicate") {
     DuplicateUserConfirm();
 }
 
-# If the user has a bug list and is processing one bug, then after
-# we process the bug we are going to show them the next bug on their
-# list.  Thus we have to make sure this bug ID is also valid,
-# since a malicious cracker might alter their cookies for the purpose
-# gaining access to bugs they are not authorized to access.
-if ( defined $::COOKIE{"BUGLIST"} && defined $::FORM{'id'} ) {
-    my @buglist = split( /:/ , $::COOKIE{"BUGLIST"} );
-    my $idx = lsearch( \@buglist , $::FORM{"id"} );
-    if ($idx < $#buglist) {
-        my $nextbugid = $buglist[$idx + 1];
-        ValidateBugID($nextbugid);
-    }
-}
-
 ######################################################################
 # End Data/Security Validation
 ######################################################################
@@ -484,6 +470,18 @@ if ($action eq Param("move-button-text")) {
 print "<TITLE>Update Bug " . join(" ", @idlist) . "</TITLE>\n";
 if (defined $::FORM{'id'}) {
     navigation_header();
+    if (defined $::next_bug) {
+        # If there is another bug, then we're going to display it,
+        # so check that its a legal bug
+        # We need to check that its a number first
+        if (!(detaint_natural($::next_bug) && CanSeeBug($::next_bug))) {
+            # This isn't OK
+            # Rather than error out (which could validly happen if there
+            # was a bug in the list whose group was changed in the meantime)
+            # just remove references to it
+            undef $::next_bug;
+        }
+    }
 }
 print "<HR>\n";
 $::query = "update bugs\nset";