From d8db706acb8316f9861abd432cfbe001dd6d0c5c Mon Sep 17 00:00:00 2001 From: Lasse Collin Date: Mon, 28 May 2012 20:42:11 +0300 Subject: [PATCH] liblzma: Fix possibility of incorrect LZMA_BUF_ERROR. lzma_code() could incorrectly return LZMA_BUF_ERROR if all of the following was true: - The caller knows how many bytes of output to expect and only provides that much output space. - When the last output bytes are decoded, the caller-provided input buffer ends right before the LZMA2 end of payload marker. So LZMA2 won't provide more output anymore, but it won't know it yet and thus won't return LZMA_STREAM_END yet. - A BCJ filter is in use and it hasn't left any unfiltered bytes in the temp buffer. This can happen with any BCJ filter, but in practice it's more likely with filters other than the x86 BCJ. Another situation where the bug can be triggered happens if the uncompressed size is zero bytes and no output space is provided. In this case the decompression can fail even if the whole input file is given to lzma_code(). A similar bug was fixed in XZ Embedded on 2011-09-19. --- src/liblzma/simple/simple_coder.c | 2 +- tests/Makefile.am | 4 +- tests/test_bcj_exact_size.c | 112 ++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 tests/test_bcj_exact_size.c diff --git a/src/liblzma/simple/simple_coder.c b/src/liblzma/simple/simple_coder.c index 9e7bc289..a02b039a 100644 --- a/src/liblzma/simple/simple_coder.c +++ b/src/liblzma/simple/simple_coder.c @@ -107,7 +107,7 @@ simple_code(lzma_coder *coder, lzma_allocator *allocator, // filtered if the buffer sizes used by the application are reasonable. const size_t out_avail = out_size - *out_pos; const size_t buf_avail = coder->size - coder->pos; - if (out_avail > buf_avail) { + if (out_avail > buf_avail || buf_avail == 0) { // Store the old position so that we know from which byte // to start filtering. const size_t out_start = *out_pos; diff --git a/tests/Makefile.am b/tests/Makefile.am index 03bf34c2..c4e17ed1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -34,7 +34,8 @@ check_PROGRAMS = \ test_stream_flags \ test_filter_flags \ test_block_header \ - test_index + test_index \ + test_bcj_exact_size TESTS = \ test_check \ @@ -42,6 +43,7 @@ TESTS = \ test_filter_flags \ test_block_header \ test_index \ + test_bcj_exact_size \ test_files.sh \ test_compress.sh diff --git a/tests/test_bcj_exact_size.c b/tests/test_bcj_exact_size.c new file mode 100644 index 00000000..cbd93405 --- /dev/null +++ b/tests/test_bcj_exact_size.c @@ -0,0 +1,112 @@ +/////////////////////////////////////////////////////////////////////////////// +// +/// \file test_bcj_exact_size.c +/// \brief Tests BCJ decoding when the output size is known +/// +/// These tests fail with XZ Utils 5.0.3 and earlier. +// +// Author: Lasse Collin +// +// This file has been put into the public domain. +// You can do whatever you want with this file. +// +/////////////////////////////////////////////////////////////////////////////// + +#include "tests.h" + + +/// Something to be compressed +static const uint8_t in[16] = "0123456789ABCDEF"; + +/// in[] after compression +static uint8_t compressed[1024]; +static size_t compressed_size = 0; + +/// Output buffer for decompressing compressed[] +static uint8_t out[sizeof(in)]; + + +static void +compress(void) +{ + // Compress with PowerPC BCJ and LZMA2. PowerPC BCJ is used because + // it has fixed 4-byte alignment which makes triggering the potential + // bug easy. + lzma_options_lzma opt_lzma2; + succeed(lzma_lzma_preset(&opt_lzma2, 0)); + + lzma_filter filters[3] = { + { .id = LZMA_FILTER_POWERPC, .options = NULL }, + { .id = LZMA_FILTER_LZMA2, .options = &opt_lzma2 }, + { .id = LZMA_VLI_UNKNOWN, .options = NULL }, + }; + + expect(lzma_stream_buffer_encode(filters, LZMA_CHECK_CRC32, NULL, + in, sizeof(in), + compressed, &compressed_size, sizeof(compressed)) + == LZMA_OK); +} + + +static void +decompress(void) +{ + lzma_stream strm = LZMA_STREAM_INIT; + expect(lzma_stream_decoder(&strm, 10 << 20, 0) == LZMA_OK); + + strm.next_in = compressed; + strm.next_out = out; + + while (true) { + if (strm.total_in < compressed_size) + strm.avail_in = 1; + + const lzma_ret ret = lzma_code(&strm, LZMA_RUN); + if (ret == LZMA_STREAM_END) { + expect(strm.total_in == compressed_size); + expect(strm.total_out == sizeof(in)); + return; + } + + expect(ret == LZMA_OK); + + if (strm.total_out < sizeof(in)) + strm.avail_out = 1; + } +} + + +static void +decompress_empty(void) +{ + // An empty file with one Block using PowerPC BCJ and LZMA2. + static const uint8_t empty_bcj_lzma2[] = { + 0xFD, 0x37, 0x7A, 0x58, 0x5A, 0x00, 0x00, 0x01, + 0x69, 0x22, 0xDE, 0x36, 0x02, 0x01, 0x05, 0x00, + 0x21, 0x01, 0x00, 0x00, 0x7F, 0xE0, 0xF1, 0xC8, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x01, 0x11, 0x00, 0x3B, 0x96, 0x5F, 0x73, + 0x90, 0x42, 0x99, 0x0D, 0x01, 0x00, 0x00, 0x00, + 0x00, 0x01, 0x59, 0x5A + }; + + // Decompress without giving any output space. + uint64_t memlimit = 1 << 20; + size_t in_pos = 0; + size_t out_pos = 0; + expect(lzma_stream_buffer_decode(&memlimit, 0, NULL, + empty_bcj_lzma2, &in_pos, sizeof(empty_bcj_lzma2), + out, &out_pos, 0) == LZMA_OK); + expect(in_pos == sizeof(empty_bcj_lzma2)); + expect(out_pos == 0); +} + + +extern int +main(void) +{ + compress(); + decompress(); + decompress_empty(); + return 0; +}