krb5 commit [krb5-1.11]: Fix various integer issues

Tom Yu tlyu at MIT.EDU
Wed Dec 12 17:04:32 EST 2012


https://github.com/krb5/krb5/commit/760bcda6c627e66684b54285f744f5584b804642
commit 760bcda6c627e66684b54285f744f5584b804642
Author: Tom Yu <tlyu at mit.edu>
Date:   Wed Dec 12 16:51:02 2012 -0500

    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.
    
    Reported by Nickolai Zeldovich <nickolai at csail.mit.edu>.
    
    (cherry picked from commit d3c5450ddf0b20855e86dab41735d56c6860156b)
    
    [tlyu at mit.edu: omitted pkinit fix because it's not conservative]
    
    ticket: 7488
    version_fixed: 1.11

 src/kadmin/dbutil/dump.c            |    3 +--
 src/kdc/kdc_preauth.c               |    3 +--
 src/kdc/kdc_util.c                  |    3 ++-
 src/lib/gssapi/spnego/spnego_mech.c |    6 +++---
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/kadmin/dbutil/dump.c b/src/kadmin/dbutil/dump.c
index cbd2d47..7b515bd 100644
--- 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 29485a3..42a37a8 100644
--- 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 ea11f54..a6a53a1 100644
--- 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 812c16d..696f42d 100644
--- 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);


More information about the cvs-krb5 mailing list