[3184] in Kerberos-V5-bugs
krb5-admin/929: kadmind potential core dump, versions 1.0.x through 1.2.2
daemon@ATHENA.MIT.EDU (crawdad@fnal.gov)
Fri Mar 16 10:54:08 2001
Resent-From: gnats@rt-11.mit.edu (GNATS Management)
Resent-To: krb5-unassigned@rt-11.mit.edu
Resent-Reply-To: krb5-bugs@MIT.EDU, crawdad@gungnir.fnal.gov
Message-Id: <200103161535.JAA03408@gungnir.fnal.gov>
Date: Fri, 16 Mar 2001 09:35:31 -0600 (CST)
From: crawdad@fnal.gov
Reply-To: crawdad@gungnir.fnal.gov
To: krb5-bugs@mit.edu
Cc: krbdev@mit.edu
>Number: 929
>Category: krb5-admin
>Synopsis: Lowering pol->pw_history_num opens a potential core dump in kadmind
>Confidential: no
>Severity: serious
>Priority: low
>Responsible: krb5-unassigned
>State: open
>Class: sw-bug
>Submitter-Id: unknown
>Arrival-Date: Fri Mar 16 10:54:00 EST 2001
>Last-Modified:
>Originator: Matt Crawford
>Organization:
Fermilab
>Release: krb5-1.2.2 (and earlier)
>Environment:
Observed on Solaris 2.7, should affect all platforms
System: SunOS gungnir.fnal.gov 5.5.1 Generic_103640-24 sun4u sparc SUNW,Ultra-1
Architecture: sun4
>Description:
If a policy is modified to have a lower pw_history_num,
or a user principal has its policy changed to one with a
lower history length, and if a user affected by that change
has not "filled" the old history list but has at least as many
old_keys as the changed or new policy would record, then kadmind
will crash at that user's next password change.
>How-To-Repeat:
Create a new principal with policy with a pw_history_num of 8.
Change that principal's password three times. Now alter the
policy to have pw_history_num = 3 (or 2 or 4), or modprinc the
principal to have a different policy with that pw_history_num.
Now change the principal's password once more. Boom.
>Fix:
The problem is in lib/kadm5/srv/svr_principal.c add_to_history(). The
code assumes that the counter adb->old_key_next will never exceed
adb->old_key_len-1. Essentially, it was written with the assumption
that the policy under which the history list was last updated is still
in force. Under the How-To-Repeat conditions it will start freeing
key data one element beyond the end of the adb->old_keys array. This
crashes with a stack trace like:
krb5_free_key_data_contents(4de58,0,7,6,4,c2) + 30
add_to_history(4de58,ffbef2c0,ffbef2dc,ffbef21c,5,4ff78) + 20c
kadm5_chpass_principal(5d6c8,60ae0,603f8,602f0,4fa90,ffffffff) + 70c
chpass_principal_wrapper(5d6c8,60ae0,603f8,ffbef480,0,ffbef455) + 68
chpass_principal_1(ffbef480,ffbef9f4,ffbef480,fffffff8,0,ffbef4d9) + 1f0
kadm_1(ffbef9f4,5b0d0,ffbef9f0,0,0,493e1) + 49c
sunrpc_seterr_reply(ffbefac8,ff03517c,ff03517c,500b8,d,5b0d0) + 19ac
kadm_svc_run(0,4b8a8,52340,4aa5c,81010100,ff00) + 198
main(2,ffbefc84,ffbefc90,4a800,0,50620) + 1694
Possible solutions are to fix up the history for all affected
principals when the policy changes (rather impractical!) or to make
fewer assumptions in add_to_history() and fix the list when the next
password change occurs.
The following patch should do the latter. I have tried to trigger the
same crash after applying this patch and found that the history list
is cleanly trimmed, with the most recent keys retained.
Index: src/lib/kadm5/srv/svr_principal.c
===================================================================
RCS file: /cvs/cd/kerberos/src/lib/kadm5/srv/svr_principal.c,v
retrieving revision 1.2
diff -c -r1.2 svr_principal.c
*** svr_principal.c 1999/06/17 01:36:31 1.2
--- svr_principal.c 2001/03/16 01:43:07
***************
*** 1001,1006 ****
--- 1001,1038 ----
memset(&adb->old_keys[adb->old_key_len],0,sizeof(osa_pw_hist_ent));
adb->old_key_len++;
+ } else if (adb->old_key_len > pol->pw_history_num-1) {
+ /*
+ * The policy must have changed! Shrink the array.
+ * Can't simply realloc() down, since it might be wrapped.
+ * To understand the arithmetic below, note that we are
+ * copying into new positions 0 .. N-1 from old positions
+ * old_key_next-N .. old_key_next-1, modulo old_key_len,
+ * where N = pw_history_num - 1 is the length of the
+ * shortened list. Matt Crawford, FNAL
+ */
+ int j;
+ histp = (osa_pw_hist_ent *)
+ malloc((pol->pw_history_num - 1) * sizeof (osa_pw_hist_ent));
+ if (histp) {
+ for (i = 0; i < pol->pw_history_num-1; i++) {
+ j = (i + adb->old_key_next - (pol->pw_history_num-1))
+ % adb->old_key_len;
+ histp[i] = adb->old_keys[j];
+ }
+ /* Now free the ones we don't keep (the oldest ones) */
+ for (i = 0; i < adb->old_key_len - (pol->pw_history_num-1); i++)
+ for (j = 0; j < adb->old_keys[i].n_key_data; j++)
+ krb5_free_key_data_contents(context,
+ &adb->old_keys[i].key_data[j]);
+ free((void *)adb->old_keys);
+ adb->old_keys = histp;
+ adb->old_key_len = pol->pw_history_num - 1;
+ adb->old_key_next = 0;
+ } else {
+ /* All is not lost! (But I dare you to exercise this path.) */
+ adb->old_key_next %= adb->old_key_len;
+ }
}
/* free the old pw history entry if it contains data */
>Audit-Trail:
>Unformatted:
