Bug 347277: Entering an non-integer sortkey crashes editclassifications.cgi - Patch by Frédéric Buclin <LpSolit@gmail.com> r=bkor a=justdave

2 files changed, 19 insertions, 5 deletions

diff --git a/editclassifications.cgi b/editclassifications.cgi
index 6d75b67d2..026f1b3ab 100755
--- a/editclassifications.cgi
+++ b/editclassifications.cgi
@@ -106,13 +106,17 @@ if ($action eq 'new') {
         ThrowUserError("classification_already_exists",
                        { name => $classification->name });
     }
-    
+
     my $description = trim($cgi->param('description')  || '');
+
     my $sortkey = trim($cgi->param('sortkey') || 0);
+    my $stored_sortkey = $sortkey;
+    detaint_natural($sortkey)
+      || ThrowUserError('classification_invalid_sortkey', {'name' => $class_name,
+                                                           'sortkey' => $stored_sortkey});
 
     trick_taint($description);
     trick_taint($class_name);
-    detaint_natural($sortkey);
 
     # Add the new classification.
     $dbh->do("INSERT INTO classifications (name, description, sortkey)
@@ -203,12 +207,18 @@ if ($action eq 'update') {
     $class_name || ThrowUserError("classification_not_specified");
 
     my $class_old_name = trim($cgi->param('classificationold') || '');
-    my $description    = trim($cgi->param('description')       || '');
-    my $sortkey        = trim($cgi->param('sortkey')           || 0);
 
     my $class_old =
         Bugzilla::Classification::check_classification($class_old_name);
 
+    my $description = trim($cgi->param('description') || '');
+
+    my $sortkey = trim($cgi->param('sortkey') || 0);
+    my $stored_sortkey = $sortkey;
+    detaint_natural($sortkey)
+      || ThrowUserError('classification_invalid_sortkey', {'name' => $class_old->name,
+                                                           'sortkey' => $stored_sortkey});
+
     $dbh->bz_lock_tables('classifications WRITE');
 
     if ($class_name ne $class_old->name) {
@@ -235,7 +245,6 @@ if ($action eq 'update') {
     }
 
     if ($sortkey ne $class_old->sortkey) {
-        detaint_natural($sortkey);
         $dbh->do("UPDATE classifications SET sortkey = ?
                   WHERE id = ?", undef,
                  ($sortkey, $class_old->id));
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index bd50bdcd7..c1bb099cf 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -261,6 +261,11 @@
     The classification '[% classification FILTER html %]' does not exist
     for product '[% product FILTER html %]'.
 
+  [% ELSIF error == "classification_invalid_sortkey" %]
+    [% title = "Invalid Sortkey for Classification" %]
+    The sortkey <em>[% sortkey FILTER html %]</em> for the '[% name FILTER html %]'
+    classification is invalid. It must be a positive integer.
+
   [% ELSIF error == "classification_not_deletable" %]
     [% title = "Default Classification Can Not Be Deleted" %]
     You can not delete the default classification