segfault in krb5_c_string_to_key()

[John Hascall]

> This bug has not been addressed.  Please submit it to krb5-bugs at mit.edu.
> I agree that it is inappropriate for mit_des_string_to_key_int() to free 
> memory it did not allocate on error.

> Your proposed solution is the one I would use.  Include it in the bug report
> and hopefully we can address this for krb5 1.3.2.

  Then you need to make sure that there are no callers that
  assume that it does...  (which is why I lazily just fixed the
  one caller that I knew was having a problem with its behavior).

John
PS, you still need the other fixes I suggested.



> Jeffrey Altman
> 
> 
> Will Fiveash wrote:
> 
> >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;
> >>    }
> >>
> >>
> >
> >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.
> >
> _______________________________________________
> krbdev mailing list             krbdev at mit.edu
> https://mailman.mit.edu/mailman/listinfo/krbdev
>