On Sep 9, 2014, at 3:11 PM, Lewis Burns <[email protected]> wrote:
> We've recently done some unit testing on open source projects. One of issues
> we've found is related to the SnifferDecompress function in the
> wiretap/ngsniffer.c file. We're unable to determine that the memory issues
> shown by valgrind can actually appear in the program due to our unfamiliarity
> with the code base. I'm sending in a small testcase to the list and hoping
> that some developers can validate or invalidate that this is a bug in the
> code.
There's one place where the code isn't doing bounds checking, which probably
accounts for
> ==5795== Invalid write of size 1
> ==5795== at 0x400798: SnifferDecompress (in
> /home/chaoqiang/workspace/se/klee/exp/a.out)
> ==5795== by 0x400B6F: main (in /home/chaoqiang/workspace/se/klee/exp/a.out)
> ==5795== Address 0x521d080 is 0 bytes after a block of size 65,536 alloc'd
> ==5795== at 0x4C2AB80: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==5795== by 0x400AE8: main (in /home/chaoqiang/workspace/se/klee/exp/a.out)
as, when I tried reproducing this, with a version of the test program built
with -g, it reported
==55119== Invalid write of size 1
==55119== at 0x100001618: SnifferDecompress (ngsniffer_noklee.c:90)
==55119== by 0x100001E32: main (ngsniffer_noklee.c:250)
==55119== Address 0x10003c150 is 0 bytes after a block of size 65,536 alloc'd
==55119== at 0xC658: malloc (vg_replace_malloc.c:295)
==55119== by 0x100001D83: main (ngsniffer_noklee.c:241)
which is the "*(pout++) = *(pin++);" in
/* Use the bits in bit_value to see what's encoded and what is
raw data */
if ( !(bit_mask & bit_value) )
{
/* bit not set - raw byte we just copy */
*(pout++) = *(pin++);
}
which, unlike other code copying to the output buffer, doesn't do a bounds
check.
I couldn't reproduce the overlap complaint, but perhaps valgrind doesn't
replace memcpy() on OS X or perhaps its replacement memcpy() doesn't check for
overlaps. I *suspect* the overlap problems come from
case 2 : /* LZ77 long strings */
/*
Low 4 bits of offset to string is the low
nybble of the
first code byte, upper 8 bits of offset is in
the next byte.
Length of string immediately follows.
Total code size: 3 bytes.
*/
offset = code_low + ((unsigned int)(*pin++) <<
4) + 3;
/* If we are already at end of input, there is
no byte
to repeat */
if ( pin >= pin_end )
{
*err = WTAP_ERR_UNC_TRUNCATED; /*
data was oddly truncated */
return ( -1 );
}
/* Check if offset would put us back past begin
of buffer */
if ( pout - offset < outbuf )
{
*err = WTAP_ERR_UNC_BAD_OFFSET;
return ( -1 );
}
/* get length from next byte, make sure it
won't overrun buf */
length = (unsigned int)(*pin++) + 16;
if ( pout + length > pout_end )
{
*err = WTAP_ERR_UNC_OVERFLOW;
return ( -1 );
}
/* Copy the string from previous text to output
position,
advance output pointer */
memcpy( pout, pout - offset, length );
pout += length;
break;
default : /* (3 to 15): LZ77 short strings */
/*
Low 4 bits of offset to string is the low
nybble of the
first code byte, upper 8 bits of offset is in
the next byte.
Length of string to repeat is overloaded into
code_type.
Total code size: 2 bytes.
*/
offset = code_low + ((unsigned int)(*pin++) <<
4) + 3;
/* Check if offset would put us back past begin
of buffer */
if ( pout - offset < outbuf )
{
*err = WTAP_ERR_UNC_BAD_OFFSET;
return ( -1 );
}
/* get length from code_type, make sure it
won't overrun buf */
length = code_type;
if ( pout + length > pout_end )
{
*err = WTAP_ERR_UNC_OVERFLOW;
return ( -1 );
}
/* Copy the string from previous text to output
position,
advance output pointer */
memcpy( pout, pout - offset, length );
pout += length;
break;
where it's *supposed* to be copying a string from earlier in the decompressed
output (thus being before the current position in the output, and thus not
overlapping), but isn't checking to make sure that the source is fully within
the already-produced portion of the output.
I'll check in fixes for both of those problems.
Thanks.
> Steps to repeat the issue:
> gcc -DRANDOM ngsniffer_noklee.c
As per the above
gcc -g -DRANDOM ngsniffer_noklee.c
may produce better debugging output from valgrind, with line numbers.
___________________________________________________________________________
Sent via: Wireshark-dev mailing list <[email protected]>
Archives: http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
mailto:[email protected]?subject=unsubscribe
