RE: security vulnerability in glibc's strfry()

[David Endler]

Hi Wolfram,

>> Description:  An exploitable buffer overflow condition exists in The
GNU
>> Projects GNU C Library (glibc) strfry(3) function. 
>> 
>> The vulnerability exists specifically because the strfry(3) function
>> does not take into account that the supplied argument string may not
be
>> null terminated.
>
>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. 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. 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.  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);
}

In examining the codebase for the strncpy() and strfry() routines we saw
that there are no differences between the GLIBC versions (with the
exception of a getpid() in creating random data within strfry()).
Another interesting note is that the above sample vulnerable program
when compiled against GLIBC 2.3.2 dynamically is exploitable; However,
when compiled statically it is not.  Any insights would be appreciated.

>> 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';
print-zephyr.c:    z_buf[sizeof(z_buf)-1] = '\0';
print-zephyr.c:    z_buf[sizeof(z_buf)-1] = '\0';

address@hidden libpcap-0.7.2]$ grep "\[sizeof" *             
pcap-dlpi.c:            if_name[sizeof(ifnet.if_name)] = '\0';
pcap-snit.c:    ifr.ifr_name[sizeof(ifr.ifr_name) - 1] = ' ';

-dave

David Endler, CISSP
Director, iDEFENSE Labs
iDEFENSE, Inc.
1875 Campus Commons Drive
Suite 210
Reston, VA 20191
voice: 703-480-5632
fax: 703-390-9456

address@hidden
www.idefense.com