Fix a memory leak by reorganizing krb5_ktf_keytab_internalize to use
authorGreg Hudson <ghudson@mit.edu>
Thu, 30 Apr 2009 17:16:20 +0000 (17:16 +0000)
committerGreg Hudson <ghudson@mit.edu>
Thu, 30 Apr 2009 17:16:20 +0000 (17:16 +0000)
the recommended exception-handling flow control.  Eliminate the check
for ktdata being null after resolution because that's not possible.
Add a check for the resolved keytab being of a different type, since
that would result in data structure corruption.

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@22295 dc483132-0cff-0310-8789-dd5450dbe970

src/lib/krb5/keytab/kt_file.c

index e2d164c93fc2e0aa0d97ab28db6092df0261e038..2aaa9b9ab281b2057967a096cc296504eec13def 100644 (file)
@@ -723,102 +723,92 @@ static krb5_error_code
 krb5_ktf_keytab_internalize(krb5_context kcontext, krb5_pointer *argp, krb5_octet **buffer, size_t *lenremain)
 {
     krb5_error_code    kret;
-    krb5_keytab                keytab;
+    krb5_keytab                keytab = NULL;
     krb5_int32         ibuf;
     krb5_octet         *bp;
     size_t             remain;
-    char               *ktname;
+    char               *ktname = NULL;
     krb5_ktfile_data   *ktdata;
     krb5_int32         file_is_open;
     krb5_int64         foff;
 
+    *argp = NULL;
     bp = *buffer;
     remain = *lenremain;
-    kret = EINVAL;
+
     /* Read our magic number */
-    if (krb5_ser_unpack_int32(&ibuf, &bp, &remain))
-       ibuf = 0;
-    if (ibuf == KV5M_KEYTAB) {
-
-       /* Get the length of the keytab name */
-       kret = krb5_ser_unpack_int32(&ibuf, &bp, &remain);
-
-       if (!kret &&
-           (ktname = (char *) malloc((size_t) (ibuf+1))) &&
-           !(kret = krb5_ser_unpack_bytes((krb5_octet *) ktname,
-                                          (size_t) ibuf,
-                                          &bp, &remain))) {
-           ktname[ibuf] = '\0';
-           kret = krb5_kt_resolve(kcontext, ktname, &keytab);
-           if (!kret) {
-               kret = ENOMEM;
-               ktdata = (krb5_ktfile_data *) keytab->data;
-               if (!ktdata) {
-                   /* XXX */
-                   keytab->data = (void *) malloc(sizeof(krb5_ktfile_data));
-                   ktdata = (krb5_ktfile_data *) keytab->data;
-                   memset(ktdata, 0, sizeof(krb5_ktfile_data));
-                   if (strchr(ktname, (int) ':'))
-                       ktdata->name = strdup(strchr(ktname, (int) ':')+1);
-                   else
-                       ktdata->name = strdup(ktname);
-               }
-               if (ktdata) {
-                   if (remain >= (sizeof(krb5_int32)*5)) {
-                       (void) krb5_ser_unpack_int32(&file_is_open,
-                                                    &bp, &remain);
-                       (void) krb5_ser_unpack_int64(&foff, &bp, &remain);
-                       (void) krb5_ser_unpack_int32(&ibuf, &bp, &remain);
-                       ktdata->version = (int) ibuf;
-
-                       (void) krb5_ser_unpack_int32(&ibuf, &bp, &remain);
-                       if (ibuf == KV5M_KEYTAB) {
-                           if (file_is_open) {
-                               int     fmode;
-                               long    fpos;
+    if (krb5_ser_unpack_int32(&ibuf, &bp, &remain) || ibuf != KV5M_KEYTAB)
+       return EINVAL;
+
+    /* Read the keytab name */
+    kret = krb5_ser_unpack_int32(&ibuf, &bp, &remain);
+    if (kret)
+       return kret;
+    ktname = malloc(ibuf + 1);
+    if (!ktname)
+       return ENOMEM;
+    kret = krb5_ser_unpack_bytes((krb5_octet *) ktname, (size_t) ibuf,
+                                &bp, &remain);
+    if (kret)
+       goto cleanup;
+    ktname[ibuf] = '\0';
+
+    /* Resolve the keytab. */
+    kret = krb5_kt_resolve(kcontext, ktname, &keytab);
+    if (kret)
+       goto cleanup;
+
+    if (keytab->ops != &krb5_ktf_writable_ops
+       && keytab->ops != &krb5_ktf_ops) {
+       kret = EINVAL;
+       goto cleanup;
+    }
+    ktdata = (krb5_ktfile_data *) keytab->data;
+
+    if (remain < (sizeof(krb5_int32)*5)) {
+       kret = EINVAL;
+       goto cleanup;
+    }
+    (void) krb5_ser_unpack_int32(&file_is_open, &bp, &remain);
+    (void) krb5_ser_unpack_int64(&foff, &bp, &remain);
+    (void) krb5_ser_unpack_int32(&ibuf, &bp, &remain);
+    ktdata->version = (int) ibuf;
+    (void) krb5_ser_unpack_int32(&ibuf, &bp, &remain);
+    if (ibuf != KV5M_KEYTAB) {
+       kret = EINVAL;
+       goto cleanup;
+    }
+
+    if (file_is_open) {
+       int     fmode;
+       long    fpos;
 
 #if !defined(_WIN32)
-                               fmode = (file_is_open >> 1) & O_ACCMODE;
+       fmode = (file_is_open >> 1) & O_ACCMODE;
 #else
-                               fmode = 0;
+       fmode = 0;
 #endif
-                               if (fmode)
-                                   kret = krb5_ktfileint_openw(kcontext,
-                                                               keytab);
-                               else
-                                   kret = krb5_ktfileint_openr(kcontext,
-                                                               keytab);
-                               if (!kret) {
-                                   fpos = foff; /* XX range check? */
-                                   if (fseek(KTFILEP(keytab), fpos,
-                                             SEEK_SET) == -1)
-                                       kret = errno;
-                               }
-                           }
-                           kret = 0;
-                       }
-                       else
-                           kret = EINVAL;
-                   }
-               }
-               if (kret) {
-                   if (keytab->data) {
-                       if (KTFILENAME(keytab))
-                           free(KTFILENAME(keytab));
-                       free(keytab->data);
-                   }
-                   free(keytab);
-               }
-               else {
-                   *buffer = bp;
-                   *lenremain = remain;
-                   *argp = (krb5_pointer) keytab;
-               }
-           }
-           free(ktname);
+       if (fmode)
+           kret = krb5_ktfileint_openw(kcontext, keytab);
+       else
+           kret = krb5_ktfileint_openr(kcontext, keytab);
+       if (kret)
+           goto cleanup;
+       fpos = foff; /* XX range check? */
+       if (fseek(KTFILEP(keytab), fpos, SEEK_SET) == -1) {
+           kret = errno;
+           goto cleanup;
        }
     }
-    return(kret);
+
+    *buffer = bp;
+    *lenremain = remain;
+    *argp = (krb5_pointer) keytab;
+cleanup:
+    if (kret != 0 && keytab)
+       krb5_kt_close(kcontext, keytab);
+    free(ktname);
+    return kret;
 }
 
 /*