diff options
| -rw-r--r-- | Bugzilla/Util.pm | 18 | ||||
| -rw-r--r-- | docs/xml/administration.xml | 2 | ||||
| -rwxr-xr-x | editmilestones.cgi | 35 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 3 |
4 files changed, 40 insertions, 18 deletions
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index 2c45e077f..70b4c6845 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -30,6 +30,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 @@ -69,6 +70,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/\&/\&/g; @@ -325,6 +336,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); @@ -393,6 +405,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 f0ccf027e..e8d70a102 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 5c9e21468..32e6790c2 100755 --- a/editmilestones.cgi +++ b/editmilestones.cgi @@ -116,6 +116,21 @@ sub CheckMilestone ($$) } } +sub CheckSortkey ($$) +{ + my ($milestone, $sortkey) = @_; + # Keep a copy in case detaint_signed() clears the sortkey + my $stored_sortkey = $sortkey; + + if (!detaint_signed($sortkey) || $sortkey < -32768 || $sortkey > 32767) { + ThrowUserError('milestone_sortkey_invalid', + {'name' => $milestone, + 'sortkey' => $stored_sortkey}); + } + + return $sortkey; +} + # # Preliminary checks: # @@ -261,13 +276,8 @@ if ($action eq 'new') { {'name' => $milestone}); } - # Need to store in case detaint_natural() clears the sortkey - my $stored_sortkey = $sortkey; - if (!detaint_natural($sortkey)) { - ThrowUserError('milestone_sortkey_invalid', - {'name' => $milestone, - 'sortkey' => $stored_sortkey}); - } + $sortkey = CheckSortkey($milestone, $sortkey); + if (TestMilestone($product, $milestone)) { ThrowUserError('milestone_already_exists', {'name' => $milestone, @@ -453,15 +463,8 @@ if ($action eq 'update') { 'milestones WRITE', 'products WRITE'); - # Need to store because detaint_natural() will delete this if - # invalid - my $stored_sortkey = $sortkey; - if ($sortkey != $sortkeyold) { - if (!detaint_natural($sortkey)) { - ThrowUserError('milestone_sortkey_invalid', - {'name' => $milestone, - 'sortkey' => $stored_sortkey}); - } + if ($sortkey ne $sortkeyold) { + $sortkey = CheckSortkey($milestone, $sortkey); trick_taint($milestoneold); diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 6c1af5b26..b7cefa9a3 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -720,7 +720,8 @@ [% ELSIF error == "milestone_sortkey_invalid" %] [% title = "Invalid Milestone Sortkey" %] The sortkey '[% sortkey FILTER html %]' for milestone ' - [% name FILTER html %]' is not a valid (positive) number. + [% name FILTER html %]' is not in the range -32768 ≤ sortkey + ≤ 32767. [% ELSIF error == "misarranged_dates" %] [% title = "Misarranged Dates" %] |