Merge pull request #2716 from antekone/bug/GH-2714/infinite-loop/1

RAR5 reader: early fail when file declares data for a dir entry

Makefile.am

@@ -955,6 +955,7 @@ libarchive_test_EXTRA_DIST=\
 	libarchive/test/test_read_format_rar5_multiple_files.rar.uu \
 	libarchive/test/test_read_format_rar5_multiple_files_solid.rar.uu \
 	libarchive/test/test_read_format_rar5_nonempty_dir_stream.rar.uu \
+	libarchive/test/test_read_format_rar5_dirdata.rar.uu \
 	libarchive/test/test_read_format_rar5_owner.rar.uu \
 	libarchive/test/test_read_format_rar5_readtables_overflow.rar.uu \
 	libarchive/test/test_read_format_rar5_sfx.exe.uu \

libarchive/archive_read_support_format_rar5.c

@@ -1703,6 +1703,33 @@ static int process_head_file_extra(struct archive_read* a,
 	return ARCHIVE_OK;
 }
 
+static int file_entry_sanity_checks(struct archive_read* a,
+	size_t block_flags, uint8_t is_dir, uint64_t unpacked_size,
+	size_t packed_size)
+{
+	if (is_dir) {
+		const int declares_data_size =
+			(int) (unpacked_size != 0 || packed_size != 0);
+
+		/* FILE entries for directories still declare HFL_DATA in block flags,
+		   even though attaching data to such blocks doesn't make much sense. */
+		if (declares_data_size) {
+			archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT,
+				"directory entries cannot have any data");
+			return ARCHIVE_FATAL;
+		}
+	} else {
+		const int declares_hfl_data = (int) ((block_flags & HFL_DATA) != 0);
+		if (!declares_hfl_data) {
+			archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT,
+					"no data found in file/service block");
+			return ARCHIVE_FATAL;
+		}
+	}
+
+	return ARCHIVE_OK;
+}
+
 static int process_head_file(struct archive_read* a, struct rar5* rar,
     struct archive_entry* entry, size_t block_flags)
 {
@@ -1718,6 +1745,7 @@ static int process_head_file(struct archive_read* a, struct rar5* rar,
 	int c_method = 0, c_version = 0;
 	char name_utf8_buf[MAX_NAME_IN_BYTES];
 	const uint8_t* p;
+	int sanity_ret;
 
 	enum FILE_FLAGS {
 		DIRECTORY = 0x0001, UTIME = 0x0002, CRC32 = 0x0004,
@@ -1761,10 +1789,6 @@ static int process_head_file(struct archive_read* a, struct rar5* rar,
 		rar->file.bytes_remaining = data_size;
 	} else {
 		rar->file.bytes_remaining = 0;
-
-		archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT,
-				"no data found in file/service block");
-		return ARCHIVE_FATAL;
 	}
 
 	if(!read_var_sized(a, &file_flags, NULL))
@@ -1781,6 +1805,13 @@ static int process_head_file(struct archive_read* a, struct rar5* rar,
 
 	rar->file.dir = (uint8_t) ((file_flags & DIRECTORY) > 0);
 
+	sanity_ret = file_entry_sanity_checks(a, block_flags, rar->file.dir,
+		unpacked_size, data_size);
+
+	if (sanity_ret != ARCHIVE_OK) {
+		return sanity_ret;
+	}
+
 	if(!read_var_sized(a, &file_attr, NULL))
 		return ARCHIVE_EOF;
 
@@ -4180,7 +4211,7 @@ static int rar5_read_data(struct archive_read *a, const void **buff,
 		 * it's impossible to perform any decompression. */
 		archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT,
 		    "Can't decompress an entry marked as a directory");
-		return ARCHIVE_FAILED;
+		return ARCHIVE_FATAL;
 	}
 
 	if(!rar->skip_mode && (rar->cstate.last_write_ptr > rar->file.unpacked_size)) {

libarchive/test/test_read_format_rar5.c

@@ -1111,6 +1111,18 @@ DEFINE_TEST(test_read_format_rar5_nonempty_dir_stream)
 	EPILOGUE();
 }
 
+DEFINE_TEST(test_read_format_rar5_nonempty_dir_data)
+{
+	PROLOGUE("test_read_format_rar5_dirdata.rar");
+
+	/* This archive is invalid. It declares a directory entry with nonzero
+	   data size. */
+
+	assertA(archive_read_next_header(a, &ae) == ARCHIVE_FATAL);
+
+	EPILOGUE();
+}
+
 DEFINE_TEST(test_read_format_rar5_fileattr)
 {
 	unsigned long set, clear, flag;

libarchive/test/test_read_format_rar5_dirdata.rar.uu

@@ -0,0 +1,6 @@
+begin 644 -
+M4F%R(1H'`0`BD'[;,`$%,#8P`0&`@("``B?GD;$U`@(+@X``"_C5%:2#``(`
+M`#"``S`P,#`P,#`P,#!);S#6KA',@]:N$?*IN;YV[8"1S>?4^`,#`R,#`P,#
+-`P,#1)'C@XX*4`O.^P``
+`
+end