[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Advice on what changes to include in krb5 upload




Hi.  I need to do an upload for a krb5 security issue.  However there
are a couple of other changes I could include.  I'd recommend all, but
it's late enough I want explicit OK before everything besides the
security fix.  I've pushed the actual diffs to the release-review branch
on the debian krb5 git.
Here's the logs

I've also attached  a diff to the actual code below. The memory leaks
are entirely in get_in_tkt.c; the security fix in kdc_authdata.c
commit 2132104b872e9254b893048fe35b0f0ff4a33c85
Author: Sam Hartman <hartmans@debian.org>
Date:   Wed Oct 13 10:47:40 2010 -0400

    s/MIt/MIT/

1	1	debian/control

commit 41244f9ba8527da7489d2630cf8285d9d8a78091
Author: Sam Hartman <hartmans@debian.org>
Date:   Wed Oct 13 10:41:50 2010 -0400

    Install doc/CHANGES only in krb5-doc, not in all packages, saves
    several megabytes on most Debian systems, Closes: #599562

3	1	debian/changelog
1	0	debian/krb5-doc.docs
1	1	debian/rules

commit d39cb90c11e768abfd9e4225f87775b009321e2e
Author: Sam Hartman <hartmans@debian.org>
Date:   Wed Oct 13 10:36:34 2010 -0400

    Fix two memory leaks in krb5_get_init_creds path; one of these memory leaks is quite common for any application such as PAM or kinit that gets initial credentials, thanks Bastian Blank, Closes: #598032

3	0	debian/changelog

commit 58da9275f0293cf87babc78cf57e83ce61c99d05
Author: Sam Hartman <hartmans@debian.org>
Date:   Tue Oct 12 20:40:29 2010 -0400

    ticket: new
    target_version: 1.9
    Subject: Fix leaks in get_init_creds interface
    
    In Debian Bug 598032, Bastian Blank points out that there are two
    leaks in the get_init_creds interface:
    
    * Free ctx->request->padata after sending the KDC request so it is not
    overwritten the next time around the loop.
    
    * If options is NULL passed into krb5_get_init_creds_init, then set up
    a non-extended options structure so that krb5_get_init_creds_free will
    free the options.

9	3	src/lib/krb5/krb/get_in_tkt.c

commit e5e9704f77be091eeda1fc781464124dad88c993
Author: Sam Hartman <hartmans@debian.org>
Date:   Tue Oct 12 12:41:28 2010 -0400

    MITKRB5-SA-2010-006 [CVE-2010-1322]: null pointer dereference in kdc_authdata.c leading to KDC crash

7	0	debian/changelog

commit 4674390460e30c0c6c8d1b9561dee991f04075b4
Author: Sam Hartman <hartmans@debian.org>
Date:   Tue Oct 12 12:37:22 2010 -0400

    MITKRB5-SA-2010-006 [CVE-2010-1322] null pointer dereference in kdc_authdata.c
    
    When the MIT krb5 KDC receives certain Kerberos TGS request messages,
    it may dereference an uninitialized pointer while processing
    authorization data, causing a crash, or in rare cases, unauthorized
    information disclosure, ticket modification, or execution of arbitrary
    code.  The crash may be triggered by legitimate requests.

4	4	src/kdc/kdc_authdata.c
diff --git a/src/kdc/kdc_authdata.c b/src/kdc/kdc_authdata.c
index b5de64d..cc44e29 100644
--- a/src/kdc/kdc_authdata.c
+++ b/src/kdc/kdc_authdata.c
@@ -495,7 +495,7 @@ merge_authdata (krb5_context context,
                 krb5_boolean copy,
                 krb5_boolean ignore_kdc_issued)
 {
-    size_t i, nadata = 0;
+    size_t i, j, nadata = 0;
     krb5_authdata **authdata = *out_authdata;
 
     if (in_authdata == NULL || in_authdata[0] == NULL)
@@ -529,16 +529,16 @@ merge_authdata (krb5_context context,
         in_authdata = tmp;
     }
 
-    for (i = 0; in_authdata[i] != NULL; i++) {
+    for (i = 0, j = 0; in_authdata[i] != NULL; i++) {
         if (ignore_kdc_issued &&
             is_kdc_issued_authdatum(context, in_authdata[i], 0)) {
             free(in_authdata[i]->contents);
             free(in_authdata[i]);
         } else
-            authdata[nadata + i] = in_authdata[i];
+            authdata[nadata + j++] = in_authdata[i];
     }
 
-    authdata[nadata + i] = NULL;
+    authdata[nadata + j] = NULL;
 
     free(in_authdata);
 
diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c
index 41059af..e46e7ac 100644
--- a/src/lib/krb5/krb/get_in_tkt.c
+++ b/src/lib/krb5/krb/get_in_tkt.c
@@ -1310,6 +1310,7 @@ krb5_init_creds_init(krb5_context context,
     int tmp;
     char *str = NULL;
     krb5_gic_opt_ext *opte;
+    krb5_get_init_creds_opt local_opts;
 
     ctx = k5alloc(sizeof(*ctx), &code);
     if (code != 0)
@@ -1332,9 +1333,12 @@ krb5_init_creds_init(krb5_context context,
     ctx->start_time = start_time;
 
     if (options == NULL) {
-        code = krb5_get_init_creds_opt_alloc(context, &options);
-        if (code != 0)
-            goto cleanup;
+        /* We initialize a non-extended options because that way the shadowed
+        flag will be sent and they will be freed when the init_creds context is
+        freed. The options will be extended and copied off the stack into
+        storage by opt_to_opte.*/
+        krb5_get_init_creds_opt_init(&local_opts);
+        options = &local_opts;
     }
 
     code = krb5int_gic_opt_to_opte(context, options,
@@ -1681,6 +1685,8 @@ init_creds_step_request(krb5_context context,
         goto cleanup;
 
 cleanup:
+    krb5_free_pa_data( context, ctx->request->padata);
+    ctx->request->padata = NULL;
     return code;
 }
 

Attachment: pgp6I6YRtmNx3.pgp
Description: PGP signature


Reply to: