Merge pull request #3073 from stoeckmann/lz4_zstd_32

lz4/zstd: Fix 32 bit platform endless loop and OOB access during bidding

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 \

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;

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.
+	 */
+	const size_t min_zstd_frame_size = 8;
 
-	// 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;
+	size_t offset_in_buffer = 0;
+	const size_t max_lookahead = 64 * 1024;
+	uint32_t magic_number;
 
-	ssize_t offset_in_buffer = 0;
-	ssize_t max_lookahead = 64 * 1024;
+	/* Zstd regular frame magic number. */
+	const uint32_t zstd_magic = 0xFD2FB528U;
 
-	// 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.
+	 */
+	const uint32_t zstd_magic_skippable_start = 0x184D2A50;
+	const uint32_t zstd_magic_skippable_mask  = 0xFFFFFFF0;
 
-	// 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 */
 
-	(void) self; // UNUSED
-
-	buffer = __archive_read_filter_ahead(filter, min_zstd_frame_size, &avail);
+	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);
+		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 zstd data.
-		if (offset_in_buffer + min_zstd_frame_size > avail) {
-			if (offset_in_buffer + min_zstd_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 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);

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

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));
+}

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