[2780] in Kerberos-V5-bugs
krb5-libs/467: AFS string_to_key bounds problems...
daemon@ATHENA.MIT.EDU (epeisach@MIT.EDU)
Fri Aug 29 21:32:15 1997
Resent-From: gnats@rt-11.MIT.EDU (GNATS Management)
Resent-To: krb5-unassigned@RT-11.MIT.EDU
Resent-Reply-To: krb5-bugs@MIT.EDU, epeisach@MIT.EDU
Date: Fri, 29 Aug 1997 21:31:47 -0400
From: epeisach@MIT.EDU
Reply-To: epeisach@MIT.EDU
To: krb5-bugs@MIT.EDU
>Number: 467
>Category: krb5-libs
>Synopsis: AFS salt type array overflow problems
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: krb5-unassigned
>State: open
>Class: sw-bug
>Submitter-Id: unknown
>Arrival-Date: Fri Aug 29 21:32:01 EDT 1997
>Last-Modified:
>Originator: Ezra Peisach
>Organization:
MIT
>Release: 1.0++
>Environment:
System: OSF1 kangaroo.mit.edu V4.0 464 alpha
Machine: alpha
>Description:
Discovered using purify with kadmind and adding principals to
the database. Supported_enctypes in kdc.conf include des:afs3.
Ok - here is the code pathway... It is a bit convoluted....
add_key_pwd (kdb_cpw.c): If the salt type is an AFS key, the
realm of the principal is krb5_copy_data into temporary storage
and the length set to -1;
mit_des_string_to_key takes a salt (krb5_data *). If the
length is -1, it then calls mit_afs_string_to_key.
mit_afs_string_to_key assigns the realm to the salt->data portion.
It then wants to use strlen on the field to determine
the length of the realm.
But oops - a krb5_data element does not have to NULL terminated!!
Therefore the strlen walks past the copied data by one. I guess with
my testing there has been a '0' at the end, but there does not have to
be one.
So the question is what to do?
I believe the right thing to do is not use krb5_copy_data, but be
smart and knowing that the lower bowels of afs_mit_string_to_key will
use strlen, copy to the realmsize + 1, 0 terminate and I believe we
win!!
I am willing to entertain other suggestions?
>How-To-Repeat:
Run purify and add keys to the database.
>Fix:
About line 395 in kdb_cpw.c:
krb5_data * saltdata;
if (retval = krb5_copy_data(context, krb5_princ_realm(context,
db_entry->princ), &saltdata))
return(retval);
key_salt.data = *saltdata;
key_salt.data.length = -1; /*length actually used below...*/
krb5_xfree(saltdata);
Change the copy_data to malloc(size+1), memcpy, copy, add the
0.... And comment why we do this crap...
If none have a better idea, I will go with this...
Ezra
>Audit-Trail:
>Unformatted:
