segfault in krb5_c_string_to_key()

[Will Fiveash]

On Mon, Jan 19, 2004 at 01:22:51PM -0600, John Hascall wrote:
> 
> 
> Q: Is this fixed in a later version?
> 
> 
> In Krb5 1.2.6 when calling krb5_get_init_creds_password()
> with krb5_prompter_posix() and you enter no password (just
> press enter) you get a segfault in krb5_c_string_to_key()
> because when it does:
> 
>     if ((ret = ((*(krb5_enctypes_list[i].str2key))(enc, string, salt, key)))) {
>         memset(key->contents, 0, keylength);
>         free(key->contents);
>     }
> 
> key->contents is NULL, because when it called krb5_des_string_to_key(),
> it in turn called mit_des_string_to_key_int() which did:
> 
> 	...
>     length = data->length + salt->length;	/* 0 + 0 */
> 	...
> 
>     copystr = malloc((size_t) length);		/* returns NULL on some O.S. */
>     if (!copystr) {
>         free(keyblock->contents);		/* already freed and */
>         keyblock->contents = 0;			/* <=== made NULL here */
>         return ENOMEM;
>     }
> 
> 
> Resulting in:
> 
> 
> Segmentation fault at >*[_OtsFill, 0x3ff800d7808]        stq     r18, 0(r16)
> (dbx) where
> >  0 _OtsFill(0x120065854, 0x11fffd868, 0x11fffd880, 0x11fffda08, 0x14000c020)
> 	[0x3ff800d7808]
>    1 krb5_c_string_to_key(context = 0x14001ed00, enctype = 1,
> 	string = 0x11fffda08, salt = 0x11fffd880, key = 0x11fffd868)
> 	["string_to_key.c":63, 0x120065878]
>    2 krb5_get_as_key_password(context = 0x14001ed00, client = 0x1400215a0,
> 	etype = 1, prompter = 0x120022e98, prompter_data = (nil),
> 	salt = 0x11fffd880, as_key = 0x11fffd868, gak_data = 0x11fffda08)
> 	["gic_pwd.c":77, 0x120023824]
>    3 krb5_get_init_creds(context = 0x14001ed00, creds = 0x11fffe7e0,
> 	client = 0x1400215a0, prompter = 0x120022e98, prompter_data = (nil),
> 	start_time = 0, in_tkt_service = (nil), options = 0x11fffe798,
> 	gak_fct = 0x1200235e0, gak_data = 0x11fffda08, use_master = 0,
> 	as_reply = 0x11fffda18) ["get_in_tkt.c":1010, 0x12002aa38]
>    4 krb5_get_init_creds_password(context = 0x14001ed00, creds = 0x11fffe7e0,
> 	client = 0x1400215a0, password = (nil), prompter = 0x120022e98,
> 	data = (nil), start_time = 0, in_tkt_service = (nil),
> 	options = 0x11fffe798) ["gic_pwd.c":132, 0x1200239b4]
> 
> If a fix has not already happened, this is probably most portable:
> 
>     if ((ret = ((*(krb5_enctypes_list[i].str2key))(enc, string, salt, key)))) {
>         if (key->contents) {				/* ADD ME */
>             memset(key->contents, 0, keylength);
>             free(key->contents);			/* ADD ME */
>         }
>     }

My question is whether it is reasonable in mit_des_string_to_key_int()
to do:

    copystr = malloc((size_t) length);
    if (!copystr) {
        free(keyblock->contents);
        keyblock->contents = 0;
        return ENOMEM;
    }

when keyblock->contents was not allocated in this function.  Seems to me
that the fix is:

    copystr = malloc((size_t) length);
    if (!copystr)
        return ENOMEM;

and leave krb5_c_string_to_key() as is.
-- 
Will Fiveash
Sun Microsystems Inc.
Austin, TX, USA (TZ=CST6CDT)
GPG PubKey ID:0x7D31DC39