From: lpsolit%gmail.com <>
Date: Wed, 4 May 2005 02:44:58 +0000 (+0000)
Subject: Bug 279303: Negative numbers are rejected as invalid sortkeys for milestones - Patch... 
X-Git-Tag: bugzilla-2.18.1~6
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7335ebc882072e16bfdb320c458a291918748850;p=thirdparty%2Fbugzilla.git

Bug 279303: Negative numbers are rejected as invalid sortkeys for milestones - Patch by Peter D. Stout <pds@edgedynamics.com> r=LpSolit a=justdave
---

diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index 0328c4f868..926b92fcee 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -29,6 +29,7 @@ use strict;
 
 use base qw(Exporter);
 @Bugzilla::Util::EXPORT = qw(is_tainted trick_taint detaint_natural
+                             detaint_signed
                              html_quote url_quote value_quote xml_quote
                              css_class_quote
                              lsearch max min
@@ -60,6 +61,16 @@ sub detaint_natural {
     return (defined($_[0]));
 }
 
+sub detaint_signed {
+    $_[0] =~ /^([-+]?\d+)$/;
+    $_[0] = $1;
+    # Remove any leading plus sign.
+    if (defined($_[0]) && $_[0] =~ /^\+(\d+)$/) {
+        $_[0] = $1;
+    }
+    return (defined($_[0]));
+}
+
 sub html_quote {
     my ($var) = (@_);
     $var =~ s/\&/\&amp;/g;
@@ -196,6 +207,7 @@ Bugzilla::Util - Generic utility functions for bugzilla
   $rv = is_tainted($var);
   trick_taint($var);
   detaint_natural($var);
+  detaint_signed($var);
 
   # Functions for quoting
   html_quote($var);
@@ -256,6 +268,12 @@ This routine detaints a natural number. It returns a true value if the
 value passed in was a valid natural number, else it returns false. You
 B<MUST> check the result of this routine to avoid security holes.
 
+=item C<detaint_signed($num)>
+
+This routine detaints a signed integer. It returns a true value if the
+value passed in was a valid signed integer, else it returns false. You
+B<MUST> check the result of this routine to avoid security holes.
+
 =back
 
 =head2 Quoting
diff --git a/docs/xml/administration.xml b/docs/xml/administration.xml
index f0ccf027e9..e8d70a1028 100644
--- a/docs/xml/administration.xml
+++ b/docs/xml/administration.xml
@@ -672,7 +672,7 @@
       <listitem>
         <para>Enter the name of the Milestone in the "Milestone" field. You
         can optionally set the "sortkey", which is a positive or negative
-        number (-255 to 255) that defines where in the list this particular
+        number (-32768 to 32767) that defines where in the list this particular
         milestone appears. This is because milestones often do not 
         occur in alphanumeric order For example, "Future" might be
         after "Release 1.2". Select "Add".</para>
diff --git a/editmilestones.cgi b/editmilestones.cgi
index aaec24455e..dce5e291d3 100755
--- a/editmilestones.cgi
+++ b/editmilestones.cgi
@@ -87,6 +87,21 @@ sub CheckMilestone ($$)
     }
 }
 
+sub CheckSortkey ($$)
+{
+    my ($milestone,$sortkey) = @_;
+
+    if (!detaint_signed($sortkey) || $sortkey < -32768 || $sortkey > 32767) {
+        print "The sortkey for a milestone must be a number between -32768 ";
+        print "and 32767 inclusive.  Please press\n";
+        print "<b>Back</b> and try again.\n";
+        PutTrailer();
+        exit;
+    }
+
+    return $sortkey;
+}
+
 
 #
 # Displays the form to edit a milestone
@@ -295,12 +310,9 @@ if ($action eq 'new') {
         PutTrailer($localtrailer);
         exit;
     }
-    if (!detaint_natural($sortkey)) {
-        print "The sortkey for a milestone must be a number. Please press\n";
-        print "<b>Back</b> and try again.\n";
-        PutTrailer($localtrailer);
-        exit;
-    }
+
+    $sortkey = CheckSortkey($milestone,$sortkey);
+
     if (TestMilestone($product,$milestone)) {
         print "The milestone '$milestone' already exists. Please press\n";
         print "<b>Back</b> and try again.\n";
@@ -518,13 +530,8 @@ if ($action eq 'update') {
                          milestones WRITE,
                          products WRITE");
 
-    if ($sortkey != $sortkeyold) {
-        if (!detaint_natural($sortkey)) {
-            print "The sortkey for a milestone must be a number. Please press\n";
-            print "<b>Back</b> and try again.\n";
-            PutTrailer($localtrailer);
-            exit;
-        }
+    if ($sortkey ne $sortkeyold) {
+        $sortkey = CheckSortkey($milestone,$sortkey);
         SendSQL("UPDATE milestones SET sortkey=$sortkey
                  WHERE product_id=" . $product_id . "
                    AND value=" . SqlQuote($milestoneold));