Fix read overflow in KDC sort_pa_data()

sort_pa_data() could read past the end of pa_order if all preauth
systems in the table have the PA_REPLACES_KEY flag, causing a
dereference of preauth_systems[-1].  This situation became possible
after commit fea1a488924faa3938ef723feaa1ff12d22a91ff with the
elimination of static_preauth_systems; before that there were always
table entries which did not have PA_REPLACES_KEY set.

Fix this bug by removing the loop to count n_key_replacers, and
instead get the count from the prior loop by stopping once we move all
of the key-replacing modules to the front.

diff --git a/src/kdc/kdc_preauth.c b/src/kdc/kdc_preauth.c
index 6f34dc289587117d1baa1946427da66439145cf6..fdf67d9d8dc90e1b032a91b791fe50a48f07dcb0 100644 (file)
--- a/src/kdc/kdc_preauth.c
+++ b/src/kdc/kdc_preauth.c
@@ -598,17 +598,18 @@ sort_pa_order(krb5_context context, krb5_kdc_req *request, int *pa_order)
                 break;
             }
         }
+        /* If we didn't find one, we have moved all of the key-replacing
+         * modules, and i is the count of those modules. */
+        if (j == n_repliers)
+            break;
     }
+    n_key_replacers = i;
 
     if (request->padata != NULL) {
         /* Now reorder the subset of modules which replace the key,
          * bubbling those which handle pa_data types provided by the
          * client ahead of the others.
          */
-        for (i = 0; preauth_systems[pa_order[i]].flags & PA_REPLACES_KEY; i++) {
-            continue;
-        }
-        n_key_replacers = i;
         for (i = 0; i < n_key_replacers; i++) {
             if (pa_list_includes(request->padata,
                                  preauth_systems[pa_order[i]].type))