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>.

(cherry picked from commit d3c5450ddf0b20855e86dab41735d56c6860156b)

[tlyu@mit.edu: omitted pkinit and kdb5_util fixes because they're not
conservative]

ticket: 7545 (new)
version_fixed: 1.10.4
status: resolved

diff --git a/src/kdc/kdc_preauth.c b/src/kdc/kdc_preauth.c
index d4ece3f0d1015ffb061b904331f949d3478153da..320bb38bf41c69dc7e724839670903bd9c725a12 100644 (file)
--- a/src/kdc/kdc_preauth.c
+++ b/src/kdc/kdc_preauth.c
@@ -507,11 +507,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 94dad3a6560e11051f7b550a6252dcb4e7fbdb9f..0e0af5a72fb2801517fb26f972772ce0532a1bea 100644 (file)
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -1118,9 +1118,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 8665d4f222e48103d48d5b5f0efc709a1c55587e..f916e49d0b4bfcb7e4976ab46ff37e15b6534ef8 100644 (file)
--- a/src/lib/gssapi/spnego/spnego_mech.c
+++ b/src/lib/gssapi/spnego/spnego_mech.c
@@ -3957,7 +3957,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;
 
@@ -3981,7 +3981,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);
@@ -3998,7 +3998,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);