Fix various integer issues

In kdc_util.c and spnego_mech.c, error returns from ASN.1 length
functions could be ignored because they were assigned to unsigned
values.  In spnego_mech.c, two buffer size checks could be rewritten
to reduce the likelihood of pointer overflow.  In dump.c and
kdc_preauth.c, calloc() could be used to simplify the code and avoid
multiplication overflow.  In pkinit_clnt.c, the wrong value was
checked for a null result from malloc(), and the code could be
simplified.

Reported by Nickolai Zeldovich <nickolai@csail.mit.edu>.

ticket: 7488

diff --git a/src/kadmin/dbutil/dump.c b/src/kadmin/dbutil/dump.c
index cbd2d477a4f248320b6b8d8bd1894818dbcdcef1..7b515bd7021240c6c4e48d2f9775c3603996a5fd 100644 (file)
--- a/src/kadmin/dbutil/dump.c
+++ b/src/kadmin/dbutil/dump.c
@@ -2109,7 +2109,7 @@ process_k5beta6_record(char *fname, krb5_context kcontext, FILE *filep,
     dbentry->n_tl_data = t3;
 
     /* Get memory for key list */
-    if (t4 && (kp = malloc(t4*sizeof(krb5_key_data))) == NULL)
+    if (t4 && (kp = calloc(t4, sizeof(krb5_key_data))) == NULL)
         goto cleanup;
 
     /* Get memory for extra data */
@@ -2121,7 +2121,6 @@ process_k5beta6_record(char *fname, krb5_context kcontext, FILE *filep,
     dbentry->e_length = t5;
 
     if (kp != NULL) {
-        memset(kp, 0, t4*sizeof(krb5_key_data));
         dbentry->key_data = kp;
         kp = NULL;
     }

diff --git a/src/kdc/kdc_preauth.c b/src/kdc/kdc_preauth.c
index 29485a34dd0660b90c8c287fb0ffd5d618a99f33..42a37a848beb77af9a74ba47b1627566a539a72c 100644 (file)
--- a/src/kdc/kdc_preauth.c
+++ b/src/kdc/kdc_preauth.c
@@ -470,11 +470,10 @@ client_keys(krb5_context context, krb5_kdcpreauth_rock rock,
     krb5_key_data *entry_key;
     int i, k;
 
-    keys = malloc(sizeof(krb5_keyblock) * (request->nktypes + 1));
+    keys = calloc(request->nktypes + 1, sizeof(krb5_keyblock));
     if (keys == NULL)
         return ENOMEM;
 
-    memset(keys, 0, sizeof(krb5_keyblock) * (request->nktypes + 1));
     k = 0;
     for (i = 0; i < request->nktypes; i++) {
         entry_key = NULL;

diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index ea11f54d104d0dc23a2a462f47a82f1269e29aa9..a6a53a1c91a72760493109c76447565a389c9304 100644 (file)
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -842,9 +842,10 @@ fetch_asn1_field(unsigned char *astream, unsigned int level,
                     /* return length and data */
                     astream++;
                     savelen = *astream;
-                    if ((data->length = asn1length(&astream)) < 0) {
+                    if ((length = asn1length(&astream)) < 0) {
                         return(-1);
                     }
+                    data->length = length;
                     /* if the field length is indefinite, we will have to subtract two
                        (terminating octets) from the length returned since we don't want
                        to pass any info from the "wrapper" back.  asn1length will always return

diff --git a/src/lib/gssapi/spnego/spnego_mech.c b/src/lib/gssapi/spnego/spnego_mech.c
index 812c16df6ddabac67f3106e5c96f7ee0b2b10256..696f42df5a0673ff4d51a7b5404170a2bca48ca6 100644 (file)
--- a/src/lib/gssapi/spnego/spnego_mech.c
+++ b/src/lib/gssapi/spnego/spnego_mech.c
@@ -3998,7 +3998,7 @@ g_verify_neg_token_init(unsigned char **buf_in, unsigned int cur_size)
 {
        unsigned char *buf = *buf_in;
        unsigned char *endptr = buf + cur_size;
-       unsigned int seqsize;
+       int seqsize;
        int ret = 0;
        unsigned int bytes;
 
@@ -4022,7 +4022,7 @@ g_verify_neg_token_init(unsigned char **buf_in, unsigned int cur_size)
                /*
                 * Make sure we have the entire buffer as described
                 */
-               if (buf + seqsize > endptr)
+               if (seqsize > endptr - buf)
                        return (G_BAD_TOK_HEADER);
        } else {
                return (G_BAD_TOK_HEADER);
@@ -4039,7 +4039,7 @@ g_verify_neg_token_init(unsigned char **buf_in, unsigned int cur_size)
                /*
                 * Make sure we have the entire buffer as described
                 */
-               if (buf + bytes > endptr)
+               if (seqsize > endptr - buf)
                        return (G_BAD_TOK_HEADER);
        } else {
                return (G_BAD_TOK_HEADER);

diff --git a/src/plugins/preauth/pkinit/pkinit_clnt.c b/src/plugins/preauth/pkinit/pkinit_clnt.c
index 7a069c1ed6c95fd5ef2a14994eb1b020e326bce1..75b97c6a803c775239e5f3743f46a61ec34ef90e 100644 (file)
--- a/src/plugins/preauth/pkinit/pkinit_clnt.c
+++ b/src/plugins/preauth/pkinit/pkinit_clnt.c
@@ -1406,40 +1406,21 @@ pkinit_client_plugin_fini(krb5_context context, krb5_clpreauth_moddata moddata)
 static krb5_error_code
 add_string_to_array(krb5_context context, char ***array, const char *addition)
 {
-    char **out = NULL;
-
-    if (*array == NULL) {
-        out = malloc(2 * sizeof(char *));
-        if (out == NULL)
-            return ENOMEM;
-        out[1] = NULL;
-        out[0] = strdup(addition);
-        if (out[0] == NULL) {
-            free(out);
-            return ENOMEM;
-        }
-    } else {
-        int i;
-        char **a = *array;
-        for (i = 0; a[i] != NULL; i++);
-        out = malloc( (i + 2) * sizeof(char *));
-        if (out == NULL)
-            return ENOMEM;
-        for (i = 0; a[i] != NULL; i++) {
-            out[i] = a[i];
-        }
-        out[i++] = strdup(addition);
-        if (out == NULL) {
-            free(out);
-            return ENOMEM;
-        }
-        out[i] = NULL;
-        free(*array);
-    }
-    *array = out;
+    char **a = *array;
+    size_t len;
 
+    for (len = 0; a != NULL && a[len] != NULL; len++);
+    a = realloc(a, (len + 2) * sizeof(char *));
+    if (a == NULL)
+        return ENOMEM;
+    *array = a;
+    a[len] = strdup(addition);
+    if (a[len] == NULL)
+        return ENOMEM;
+    a[len + 1] = NULL;
     return 0;
 }
+
 static krb5_error_code
 handle_gic_opt(krb5_context context,
                pkinit_context plgctx,