LZ4_uncompress_unknownOuptutSize() can read outside the input buffer

[GoogleCodeExporter]

Hi,

The header for LZ4_Decompress() states:

// Note : The decoding functions LZ4_uncompress() and 
LZ4_uncompress_unknownOutputSize()
//              are safe against "buffer overflow" attack type.
//              They will never write nor read outside of the provided input 
and output buffers.
//              A corrupted input will produce an error result, a negative int, 
indicating the position of the error within input stream.

However, the following demo program shows that this is not always the case:

#include <stdio.h>
#include <string.h>
#include "lz4.h"

int main(int argc, char** argv)
{
        char *input, *output;

        input = (char *)malloc(3);
        input[0] = 0xf0;
        input[1] = 0xfe;
        input[2] = 0x00;

        output = (char *)malloc(4096);

        int ret = LZ4_uncompress_unknownOutputSize(input, output, 3, 4096);
        printf("%d\n", ret);
        return 0;
}

Running it with Valgrind reveals:

==26795== Invalid read of size 8
==26795==    at 0x400750: LZ4_uncompress_unknownOutputSize 
(/home/sesse/lz4-read-only/lz4.c:744)
==26795==    by 0x400667: main (/home/sesse/lz4-read-only/democase.c:16)
==26795==  Address 0x51b6072 is 2 bytes inside a block of size 3 alloc'd
==26795==    at 0x4C2A4D6: malloc 
(/home/samsonov/valgrind-variant/valgrind-test/coregrind/m_replacemalloc/vg_repl
ace_malloc.c:263)
==26795==    by 0x40063A: main (/home/sesse/lz4-read-only/democase.c:9)
==26795== 
==26795== Invalid read of size 8
==26795==    at 0x400761: LZ4_uncompress_unknownOutputSize 
(/home/sesse/lz4-read-only/lz4.c:744)
==26795==    by 0x400667: main (/home/sesse/lz4-read-only/democase.c:16)
==26795==  Address 0x51b607a is 7 bytes after a block of size 3 alloc'd
==26795==    at 0x4C2A4D6: malloc 
(/home/samsonov/valgrind-variant/valgrind-test/coregrind/m_replacemalloc/vg_repl
ace_malloc.c:263)
==26795==    by 0x40063A: main (/home/sesse/lz4-read-only/democase.c:9)

This is with LZ4 as of r57, 64-bit x86, GCC 4.4.3.

Original issue reported on code.google.com by sgunder...@bigfoot.com on 1 Mar 2012 at 11:34

[GoogleCodeExporter]

Thanks for pointing that out. I will look into it.

Original comment by yann.col...@gmail.com on 1 Mar 2012 at 12:18

• Changed state: Accepted

[GoogleCodeExporter]

Please find in attached file a proposed correction which should solve the 
reported problem.

Rgds

Original comment by yann.col...@gmail.com on 1 Mar 2012 at 2:14

Attachments:

• lz4rc58.zip

[GoogleCodeExporter]

That's a pretty large number of diffs against the current Subversion release, 
including lots of changes to things like expect(). Is this really meant as a 
minimal fix?

Original comment by sgunder...@bigfoot.com on 1 Mar 2012 at 2:18

[GoogleCodeExporter]

Well, lz4 was already updated and heading towards r58, using __builtin_expect 
ability, when this bug report was introduced.

So indeed this version combines both.

Original comment by yann.col...@gmail.com on 1 Mar 2012 at 2:41

[GoogleCodeExporter]

OK, well, in any case, the lz4.c version you sent seems to have fixed the 
problem.

It's a lot simpler to follow code that restricts each Subversion commit to one 
distinct change, though!

Original comment by sgunder...@bigfoot.com on 1 Mar 2012 at 2:43

[GoogleCodeExporter]

Thanks for report Steinar. Testings will continue a bit, and r58 will then be 
released shortly.

Original comment by yann.col...@gmail.com on 1 Mar 2012 at 2:53

• Changed state: Fixed