Re: security vulnerability in glibc's strfry()

[wmglo]

Hi,

> >Is this a joke?  A "string" in C _is_ null-terminated, period.  _All_
> >the str... functions require null-terminated strings unless stated
> >otherwise.  If strfry would work with non-terminated buffers, it would
> >be called memfry (and would obviously need a size argument).
> 
> Not true.

What is not true?  I stand by my statement above.

> Many a times a progammer will (sloppily) fill a stack buffer
> with data to the brim of the buffer and forget to null terminate the
> string.

That is a buggy program then, as soon as you use any str...() function
on the buffer.

> Agreed, programmers should always ensure that their strings are
> indeed null terminated. The fact of the matter is that the following
> sample program when compiled against GLIBC v2.2 is not vulnerable to
> exploitation; However, when compiled against GLIBC >= 2.3.2 it _IS_
> vulnerable to exploitation.

So bug-compatibility of glibc has perhaps decreased going from 2.2 to
2.3.2.  Big deal -- this has happened before and will surely continue
to happen in the future.

> The reason we're reporting this issue to
> you and the glibc group is we are unsure where or if there is a larger
> exploitable bug that was introduced which allows for this exploitation,
> when the older version does not:
> 
> int main(int argc, char *argv[])
> {
>     char buffer[256], buffer2[256];
> 
>     strncpy(buffer, argv[1], sizeof(buffer));
>     strfry(buffer);
> }

Sorry, I don't think any glibc developer is interested in analyzing
why this broken code is now exploitable or why it wasn't before -- it
remains broken code that should simply be fixed.

> >> Workaround:  Applications requiring the usage of strfry(3) should
> ensure
> >> that strings passed to strfry(3) are NULL terminated. Example: 
> >> 
> >> buffer[sizeof(buffer) - 1] = "\0";
> >> strfry(buffer);  
> 
> >Yeah, right.  Hint: this should't even compile, and if it would, you'd
> >have a massive bug.
> 
> Simple typo. Should read '\0' and such method is commonly used
> especially in packet parsing code:
> 
> address@hidden tcpdump-3.7.2]$ grep "\[sizeof" * | grep "\-1"
> print-rip.c:                    buf[sizeof(buf)-1] = '\0';

Sure -- but this is not just a "commonly used method" or a
"workaround", instead it is a _required_ measure for program
correctness.

Regards,
Wolfram.