From: Martin Matuška Date: Tue, 2 Jun 2026 13:05:42 +0000 (+0200) Subject: Merge pull request #3073 from stoeckmann/lz4_zstd_32 X-Git-Tag: v3.8.8~62 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e91dffdc92f053bbae75acb06dc4215844c4ff1c;p=thirdparty%2Flibarchive.git Merge pull request #3073 from stoeckmann/lz4_zstd_32 lz4/zstd: Fix 32 bit platform endless loop and OOB access during bidding (cherry picked from commit 6ac02f95216103b8e71730fc80bdcfb874ce22e4) --- diff --git a/Makefile.am b/Makefile.am index 8f31fa169..3b1268dbe 100644 --- a/Makefile.am +++ b/Makefile.am @@ -477,6 +477,7 @@ libarchive_test_SOURCES= \ libarchive/test/test_read_filter_lzop_multiple_parts.c \ libarchive/test/test_read_filter_program.c \ libarchive/test/test_read_filter_program_signature.c \ + libarchive/test/test_read_filter_zstd_raw.c \ libarchive/test/test_read_filter_uudecode.c \ libarchive/test/test_read_filter_uudecode_raw.c \ libarchive/test/test_read_format_7zip.c \ @@ -832,6 +833,7 @@ libarchive_test_EXTRA_DIST=\ libarchive/test/test_read_filter_lzop_multiple_parts.tar.lzo.uu \ libarchive/test/test_read_filter_uudecode_raw.uu \ libarchive/test/test_read_filter_uudecode_base64_raw.uu \ + libarchive/test/test_read_filter_zstd_raw_loop.uu \ libarchive/test/test_read_format_mtree_crash747.mtree.bz2.uu \ libarchive/test/test_read_format_mtree_noprint.mtree.uu \ libarchive/test/test_read_format_7zip_bcj2_bzip2.7z.uu \ diff --git a/libarchive/archive_read_support_filter_lz4.c b/libarchive/archive_read_support_filter_lz4.c index 15f648102..0357543bf 100644 --- a/libarchive/archive_read_support_filter_lz4.c +++ b/libarchive/archive_read_support_filter_lz4.c @@ -144,52 +144,74 @@ lz4_reader_bid(struct archive_read_filter_bidder *self, const unsigned char *buffer; ssize_t avail; int bits_checked = 0; - ssize_t min_lz4_archive_size = 11; + const size_t min_lz4_archive_size = 11; - // LZ4 skippable frames contain a 4 byte magic number followed by - // a 4 byte frame data size, then that number of bytes of data. Regular - // frames contain a 4 byte magic number followed by a 2-14 byte frame - // header, some data, and a 3 byte end marker. - ssize_t min_lz4_frame_size = 8; + /* + * LZ4 skippable frames contain a 4 byte magic number followed by + * a 4 byte frame data size, then that number of bytes of data. + * Regular frames contain a 4 byte magic number followed by a 2-14 + * byte frame header, some data, and a 3 byte end marker. + */ + const size_t min_lz4_frame_size = 8; - ssize_t offset_in_buffer = 0; - ssize_t max_lookahead = 64 * 1024; + size_t offset_in_buffer = 0; + const size_t max_lookahead = 64 * 1024; + uint32_t magic_number; - (void)self; // UNUSED + (void)self; /* UNUSED */ - // Zstd and LZ4 skippable frame magic numbers are identical. To - // differentiate these two, we need to look for a non-skippable - // frame. + /* + * Zstd and LZ4 skippable frame magic numbers are identical. To + * differentiate these two, we need to look for a non-skippable + * frame. + */ - // Minimal lz4 archive is 11 bytes. - buffer = __archive_read_filter_ahead(filter, min_lz4_archive_size, &avail); + /* Minimal lz4 archive is 11 bytes. */ + buffer = __archive_read_filter_ahead(filter, min_lz4_archive_size, + &avail); if (buffer == NULL) return (0); - uint32_t magic_number = archive_le32dec(buffer); + magic_number = archive_le32dec(buffer); while ((magic_number & LZ4_SKIPPABLE_MASK) == LZ4_SKIPPABLE_START) { + uint32_t frame_data_size; - offset_in_buffer += 4; // Skip over the magic number + /* Skip over the magic number */ + offset_in_buffer += 4; - // Ensure that we can read another 4 bytes. - if (offset_in_buffer + 4 > avail) { - buffer = __archive_read_filter_ahead(filter, offset_in_buffer + 4, &avail); + /* Ensure that we can read another 4 bytes. */ + if (offset_in_buffer + 4 > (size_t)avail) { + buffer = __archive_read_filter_ahead(filter, + offset_in_buffer + 4, &avail); if (buffer == NULL) return (0); } - uint32_t frame_data_size = archive_le32dec(buffer + offset_in_buffer); + frame_data_size = archive_le32dec(buffer + offset_in_buffer); - // Skip over the 4 frame data size bytes, plus the value stored there. - offset_in_buffer += 4 + frame_data_size; + /* Skip over the 4 frame data size bytes */ + offset_in_buffer += 4; - // There should be at least one more frame if this is LZ4 data. - if (offset_in_buffer + min_lz4_frame_size > avail) { // TODO: should this be >= ? - if (offset_in_buffer + min_lz4_frame_size > max_lookahead) + /* Skip over the value stored there. */ + if (frame_data_size > SIZE_MAX - offset_in_buffer) + return (0); + offset_in_buffer += frame_data_size; + + /* + * There should be at least one more frame + * if this is LZ4 data. + */ + if (min_lz4_frame_size > SIZE_MAX - offset_in_buffer) + return (0); + /* TODO: should this be >= ? */ + if (offset_in_buffer + min_lz4_frame_size > (size_t)avail) { + if (offset_in_buffer + min_lz4_frame_size > + max_lookahead) return (0); - buffer = __archive_read_filter_ahead(filter, offset_in_buffer + min_lz4_frame_size, &avail); + buffer = __archive_read_filter_ahead(filter, + offset_in_buffer + min_lz4_frame_size, &avail); if (buffer == NULL) return (0); } @@ -197,8 +219,10 @@ lz4_reader_bid(struct archive_read_filter_bidder *self, magic_number = archive_le32dec(buffer + offset_in_buffer); } - // We have skipped over any skippable frames. Either a regular LZ4 frame - // follows, or this isn't LZ4 data. + /* + * We have skipped over any skippable frames. Either a regular LZ4 frame + * follows, or this isn't LZ4 data. + */ bits_checked = offset_in_buffer; buffer = buffer + offset_in_buffer; diff --git a/libarchive/archive_read_support_filter_zstd.c b/libarchive/archive_read_support_filter_zstd.c index 8dd004259..f1b4cec9e 100644 --- a/libarchive/archive_read_support_filter_zstd.c +++ b/libarchive/archive_read_support_filter_zstd.c @@ -108,55 +108,76 @@ zstd_bidder_bid(struct archive_read_filter_bidder *self, { const unsigned char *buffer; ssize_t avail; - - // Zstandard skippable frames contain a 4 byte magic number followed by - // a 4 byte frame data size, then that number of bytes of data. Regular - // frames contain a 4 byte magic number followed by a 2-14 byte frame - // header, some data, and a 3 byte end marker. - ssize_t min_zstd_frame_size = 8; - - ssize_t offset_in_buffer = 0; - ssize_t max_lookahead = 64 * 1024; - - // Zstd regular frame magic number. - uint32_t zstd_magic = 0xFD2FB528U; - - // Note: Zstd and LZ4 skippable frame magic numbers are identical. - // To differentiate these two, we need to look for a non-skippable - // frame. - uint32_t zstd_magic_skippable_start = 0x184D2A50; - uint32_t zstd_magic_skippable_mask = 0xFFFFFFF0; - - (void) self; // UNUSED - - buffer = __archive_read_filter_ahead(filter, min_zstd_frame_size, &avail); + /* + * Zstandard skippable frames contain a 4 byte magic number followed + * by a 4 byte frame data size, then that number of bytes of data. + * Regular frames contain a 4 byte magic number followed by a 2-14 + * byte frame header, some data, and a 3 byte end marker. + */ + const size_t min_zstd_frame_size = 8; + + size_t offset_in_buffer = 0; + const size_t max_lookahead = 64 * 1024; + uint32_t magic_number; + + /* Zstd regular frame magic number. */ + const uint32_t zstd_magic = 0xFD2FB528U; + + /* + * Note: Zstd and LZ4 skippable frame magic numbers are identical. + * To differentiate these two, we need to look for a non-skippable + * frame. + */ + const uint32_t zstd_magic_skippable_start = 0x184D2A50; + const uint32_t zstd_magic_skippable_mask = 0xFFFFFFF0; + + (void) self; /* UNUSED */ + + buffer = __archive_read_filter_ahead(filter, min_zstd_frame_size, + &avail); if (buffer == NULL) return (0); - uint32_t magic_number = archive_le32dec(buffer); + magic_number = archive_le32dec(buffer); - while ((magic_number & zstd_magic_skippable_mask) == zstd_magic_skippable_start) { + while ((magic_number & zstd_magic_skippable_mask) == + zstd_magic_skippable_start) { + uint32_t frame_data_size; - offset_in_buffer += 4; // Skip over the magic number + /* Skip over the magic number */ + offset_in_buffer += 4; - // Ensure that we can read another 4 bytes. - if (offset_in_buffer + 4 > avail) { - buffer = __archive_read_filter_ahead(filter, offset_in_buffer + 4, &avail); + /* Ensure that we can read another 4 bytes. */ + if (offset_in_buffer + 4 > (size_t)avail) { + buffer = __archive_read_filter_ahead(filter, + offset_in_buffer + 4, &avail); if (buffer == NULL) return (0); } - uint32_t frame_data_size = archive_le32dec(buffer + offset_in_buffer); - - // Skip over the 4 frame data size bytes, plus the value stored there. - offset_in_buffer += 4 + frame_data_size; - - // There should be at least one more frame if this is zstd data. - if (offset_in_buffer + min_zstd_frame_size > avail) { - if (offset_in_buffer + min_zstd_frame_size > max_lookahead) + frame_data_size = archive_le32dec(buffer + offset_in_buffer); + + /* Skip over the 4 frame data size bytes */ + offset_in_buffer += 4; + + /* Skip over the value stored there. */ + if (frame_data_size > SIZE_MAX - offset_in_buffer) + return (0); + offset_in_buffer += frame_data_size; + + /* + * There should be at least one more frame + * if this is zstd data. + */ + if (min_zstd_frame_size > SIZE_MAX - offset_in_buffer) + return (0); + if (offset_in_buffer + min_zstd_frame_size > (size_t)avail) { + if (offset_in_buffer + min_zstd_frame_size > + max_lookahead) return (0); - buffer = __archive_read_filter_ahead(filter, offset_in_buffer + min_zstd_frame_size, &avail); + buffer = __archive_read_filter_ahead(filter, + offset_in_buffer + min_zstd_frame_size, &avail); if (buffer == NULL) return (0); } @@ -164,8 +185,10 @@ zstd_bidder_bid(struct archive_read_filter_bidder *self, magic_number = archive_le32dec(buffer + offset_in_buffer); } - // We have skipped over any skippable frames. Either a regular zstd frame - // follows, or this isn't zstd data. + /* + * We have skipped over any skippable frames. Either a regular zstd + * frame follows, or this isn't zstd data. + */ if (magic_number == zstd_magic) return (offset_in_buffer + 4); diff --git a/libarchive/test/CMakeLists.txt b/libarchive/test/CMakeLists.txt index c67e71f3d..d3f67b584 100644 --- a/libarchive/test/CMakeLists.txt +++ b/libarchive/test/CMakeLists.txt @@ -111,6 +111,7 @@ IF(ENABLE_TEST) test_read_filter_program_signature.c test_read_filter_uudecode.c test_read_filter_uudecode_raw.c + test_read_filter_zstd_raw.c test_read_format_7zip.c test_read_format_7zip_encryption_data.c test_read_format_7zip_encryption_header.c diff --git a/libarchive/test/test_read_filter_zstd_raw.c b/libarchive/test/test_read_filter_zstd_raw.c new file mode 100644 index 000000000..5a4ea91c6 --- /dev/null +++ b/libarchive/test/test_read_filter_zstd_raw.c @@ -0,0 +1,46 @@ +/*- + * Copyright (c) 2026 Tobias Stoeckmann + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR(S) ``AS IS'' AND ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL THE AUTHOR(S) BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +#include "test.h" + +DEFINE_TEST(test_read_filter_zstd_raw_loop) +{ + struct archive *a; + + const char *name = "test_read_filter_zstd_raw_loop"; + + assert((a = archive_read_new()) != NULL); + assertEqualIntA(a, ARCHIVE_OK, archive_read_support_filter_all(a)); + assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_raw(a)); + extract_reference_file(name); + assertEqualIntA(a, ARCHIVE_OK, + archive_read_open_filename(a, name, 200)); + + /* Verify that the filter detection did NOT work. */ + assertEqualInt(archive_filter_code(a, 0), ARCHIVE_FILTER_NONE); + assertEqualString(archive_filter_name(a, 0), "none"); + + assertEqualInt(ARCHIVE_OK, archive_read_close(a)); + assertEqualInt(ARCHIVE_OK, archive_read_free(a)); +} diff --git a/libarchive/test/test_read_filter_zstd_raw_loop.uu b/libarchive/test/test_read_filter_zstd_raw_loop.uu new file mode 100644 index 000000000..932715669 --- /dev/null +++ b/libarchive/test/test_read_filter_zstd_raw_loop.uu @@ -0,0 +1,4 @@ +begin 644 test_read_filter_zstd_raw_loop +,4"I-&/C___\HM2_] +` +end