Date: Sun, 15 Oct 1995 12:55:51 +1000 From: Bruce Evans <bde@zeta.org.au> To: bde@zeta.org.au, terry@lambert.org Cc: freebsd-current@freefall.freebsd.org, jc@irbs.com Subject: Re: phkmalloc and X programs Message-ID: <199510150255.MAA01536@godzilla.zeta.org.au>
next in thread | raw e-mail | index | archive | help
>> >> xhost.c:
>> >>
>> >> namelen = strlen(name);
>> >> if ((lname = (char *)malloc(namelen)) == NULL) {
>> >> fprintf (stderr, "%s: malloc bombed in change_host\n", ProgramName);
>> >> exit (1);
>> >> }
>> >> for (i = 0; i < namelen; i++) {
>> >> lname[i] = tolower(name[i]);
>> >> }
>> >> if (!strncmp("inet:", lname, 5)) {
>> >> ...
>> >> ...
>>
>> >The only assumption in this code is that namelen is >= 5.
>>
>> Nope. Suppose lname is initially "INOT:" and name is "inet"
>Then namelen < 5 (== 4) and the code fails. I already said that that
>was the assumption. 8-).
Nope. When namelen < 5, bytes lname[namelen]..lname[4] are garbage
and the failure of the code depends on the garbage. Anyway, the original
problem was apparently in the ... code. It was neverthless easy to find
bugs in the code shown since it was more than one line long :-). Other
bugs:
1. namelen and i have bogus types, so if strlen(name) == (size_t)INT_MAX + 1,
then the for loop will exit immediately, leaving at least 32K bytes of
lname[] uninitialized.
2. tolower(name[i]) is undefined if name[i] < 0 && name[i] != EOF.
>Probably the "correct" "fix" is to change:
> if (!strncmp("inet:", lname, 5)) {
>To:
> if (namelen >= 5 && !strncmp("inet:", lname, 5)) {
Something like that.
>> >There is no assumption of numm termination on the lname string implicit
>> >in the malloc; if there were, it would be "namelen = strlen(name) + 1;".
>>
>> That may be why the author thought that termination was unnecessary.
>The author thought that the allocated area was >= 5 for any namelen,
>making an assumption about the way the malloc on his system functioned,
>such that lname[0..4] was an addressable location.
Sigh, another bug. My example only covers the case where the bytes happen
to be addressable and have contain interesting garbage.
>If the allocated area happened to contain "xxet:" and name was "in", it
>would falsely hit positive.
Same as my example.
>This is statistically highly improbable. Likely the code will function
>in common use anyway.
More probable than (1) :-).
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199510150255.MAA01536>