ZIP reader: fix memory leak when unpacking LZMA

This commit fixes a memory leak which is triggered by invalid files.
Sample test case that triggers the leak is provided by OSSFuzz #14470.

If the ZIPX file contanis an LZMA stream, and this stream is invalid,
the reader was allocating an LZMA decoding context which wasn't freed.
Later, when trying to unpack another LZMA stream, context was
re-initialized by allocating a new context and overwriting old pointers
to an unfreed memory, causing a memory leak.

After applying this commit, the LZMA stream context initialization
function will check if there is an non-freed previous context being in
use. If it exists, the reader will free the memory before allocating a
new LZMA unpacking context.

The commit also contains a test case with OSSFuzz sample #14470.

diff --git a/libarchive/archive_read_support_format_zip.c b/libarchive/archive_read_support_format_zip.c
index 6937969c1626b1fee4bf54edd90d108935a4a69f..05b73705bec67ae3c3ccecd3555195709ecf58ac 100644 (file)
--- a/libarchive/archive_read_support_format_zip.c
+++ b/libarchive/archive_read_support_format_zip.c
@@ -1444,6 +1444,11 @@ zipx_lzma_alone_init(struct archive_read *a, struct zip *zip)
        } alone_header;
 #pragma pack(pop)
 
+       if(zip->zipx_lzma_valid) {
+               lzma_end(&zip->zipx_lzma_stream);
+               zip->zipx_lzma_valid = 0;
+       }
+
        /* To unpack ZIPX's "LZMA" (id 14) stream we can use standard liblzma that
         * is a part of XZ Utils. The stream format stored inside ZIPX file is a
         * modified "lzma alone" file format, that was used by the `lzma` utility

diff --git a/libarchive/test/test_read_format_zip.c b/libarchive/test/test_read_format_zip.c
index bdd23c2ac98656650dd418ba106aacca005c5a45..e47d17cd5076d4d2003844e8adb2bcaa505fb2f4 100644 (file)
--- a/libarchive/test/test_read_format_zip.c
+++ b/libarchive/test/test_read_format_zip.c
@@ -878,3 +878,41 @@ DEFINE_TEST(test_read_format_zip_ppmd8_crash_2)
        assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a));
        assertEqualIntA(a, ARCHIVE_OK, archive_read_free(a));
 }
+
+DEFINE_TEST(test_read_format_zip_lzma_alone_leak)
+{
+       const char *refname = "test_read_format_zip_lzma_alone_leak.zipx";
+       struct archive *a;
+       struct archive_entry *ae;
+       char buf[64];
+
+       /* OSSFuzz #14470 sample file. */
+       extract_reference_file(refname);
+
+       assert((a = archive_read_new()) != NULL);
+       if(ARCHIVE_OK != archive_read_support_filter_lzma(a)) {
+               skipping("lzma reading is not fully supported on this platform");
+               archive_read_close(a);
+               return;
+       }
+
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_zip(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_open_filename(a, refname, 37));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_next_header(a, &ae));
+
+       /* Extraction of this file should fail, because the sample file is invalid.
+        * But it shouldn't crash. */
+       assertEqualIntA(a, ARCHIVE_FAILED, archive_read_data(a, buf, sizeof(buf)));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_next_header(a, &ae));
+
+       /* Extraction of this file should fail, because the sample file is invalid.
+        * But it shouldn't crash. */
+       assertEqualIntA(a, ARCHIVE_FATAL, archive_read_data(a, buf, sizeof(buf)));
+
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_free(a));
+
+       /* This testcase shouldn't produce any memory leaks. When running test
+        * suite under Valgrind or ASan, the test runner won't return with
+        * exit code 0 in case if a memory leak. */
+}

diff --git a/libarchive/test/test_read_format_zip_lzma_alone_leak.zipx.uu b/libarchive/test/test_read_format_zip_lzma_alone_leak.zipx.uu
new file mode 100644 (file)
index 0000000..5fce213
--- /dev/null
+++ b/libarchive/test/test_read_format_zip_lzma_alone_leak.zipx.uu
@@ -0,0 +1,5 @@
+begin 644 test_read_format_zip_lzma_alone_leak.zipx
+M4$L#!"`@6B`.("`@("`@("`@%0```"`@("````4``0`!`"`@(`4``"``````
+J(/\@("`@("`@("!02P,$("`@(`X@(/________\@("`@("`@(``````@
+`
+end