From 8fbdea708c2d60d7c02b82d0d9933156152d5c1c Mon Sep 17 00:00:00 2001
From: Enno Boland <g@s01.de>
Date: Wed, 16 Aug 2023 18:57:42 +0200
Subject: [PATCH] directory_iterator: fix wrong free, add tests

during one of the last refactorings of the heap constructors,
there was a bug introduced that caused a free of the wrong
pointer. This commit fixes that bug, makes the wrongly freed
pointer const to yield compiler warnings on missuse, and adds
a test to make sure it doesn't happen again.
---
 include/sqsh_directory_private.h    |  4 ++--
 lib/directory/directory_iterator.c  | 10 ++++-----
 test/directory/directory_iterator.c | 35 ++++++++++++++++++++++++++++-
 3 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/include/sqsh_directory_private.h b/include/sqsh_directory_private.h
index 18fc5b42d..2a46169e5 100644
--- a/include/sqsh_directory_private.h
+++ b/include/sqsh_directory_private.h
@@ -54,7 +54,7 @@ struct SqshDirectoryIterator {
 	/**
 	 * @privatesection
 	 */
-	struct SqshFile *file;
+	const struct SqshFile *file;
 	uint32_t remaining_size;
 
 	struct SqshMetablockReader metablock;
@@ -76,7 +76,7 @@ struct SqshDirectoryIterator {
  * @return 0 on success, a negative value on error.
  */
 SQSH_NO_EXPORT SQSH_NO_UNUSED int sqsh__directory_iterator_init(
-		struct SqshDirectoryIterator *iterator, struct SqshFile *inode);
+		struct SqshDirectoryIterator *iterator, const struct SqshFile *inode);
 
 /**
  * @internal
diff --git a/lib/directory/directory_iterator.c b/lib/directory/directory_iterator.c
index 3c46f6d1b..a455da6a4 100644
--- a/lib/directory/directory_iterator.c
+++ b/lib/directory/directory_iterator.c
@@ -54,7 +54,7 @@ static int
 load_metablock(
 		struct SqshDirectoryIterator *iterator, const uint64_t outer_offset,
 		uint32_t inner_offset) {
-	struct SqshFile *file = iterator->file;
+	const struct SqshFile *file = iterator->file;
 	struct SqshArchive *archive = file->archive;
 	const struct SqshSuperblock *superblock = sqsh_archive_superblock(archive);
 
@@ -94,7 +94,7 @@ directory_iterator_index_lookup(
 		const size_t name_len) {
 	int rv = 0;
 	struct SqshDirectoryIndexIterator index_iterator = {0};
-	struct SqshFile *file = iterator->file;
+	const struct SqshFile *file = iterator->file;
 	const uint64_t inode_ref = sqsh_file_inode_ref(file);
 	uint64_t outer_offset = sqsh_file_directory_block_start(file);
 	uint32_t inner_offset = sqsh_file_directory_block_offset(file);
@@ -175,7 +175,7 @@ sqsh_directory_iterator_lookup(
 
 int
 sqsh__directory_iterator_init(
-		struct SqshDirectoryIterator *iterator, struct SqshFile *file) {
+		struct SqshDirectoryIterator *iterator, const struct SqshFile *file) {
 	int rv = 0;
 
 	if (sqsh_file_type(file) != SQSH_FILE_TYPE_DIRECTORY) {
@@ -205,8 +205,8 @@ sqsh_directory_iterator_new(struct SqshFile *file, int *err) {
 	}
 	rv = sqsh__directory_iterator_init(iterator, file);
 	if (rv < 0) {
-		free(file);
-		file = NULL;
+		free(iterator);
+		iterator = NULL;
 	}
 out:
 	if (err != NULL) {
diff --git a/test/directory/directory_iterator.c b/test/directory/directory_iterator.c
index fa4810372..320b26ec3 100644
--- a/test/directory/directory_iterator.c
+++ b/test/directory/directory_iterator.c
@@ -124,7 +124,7 @@ iter_two_files(void) {
 			[INODE_TABLE_OFFSET] = METABLOCK_HEADER(0, 1024),
 			INODE_HEADER(1, 0, 0, 0, 0, 1),
 			INODE_BASIC_DIR(0, 1024, 0, 0),
-			[INODE_TABLE_OFFSET+2+128] = 
+			[INODE_TABLE_OFFSET+2+128] =
 			INODE_HEADER(3, 0, 0, 0, 0, 2),
 			INODE_BASIC_SYMLINK(3),
 			't', 'g', 't',
@@ -171,8 +171,41 @@ iter_two_files(void) {
 	sqsh__archive_cleanup(&archive);
 }
 
+static void
+iter_invalid_file_type(void) {
+	int rv;
+	struct SqshArchive archive = {0};
+	uint8_t payload[] = {
+			/* clang-format off */
+			SQSH_HEADER,
+			/* inode */
+			[INODE_TABLE_OFFSET] = METABLOCK_HEADER(0, 1024),
+			INODE_HEADER(3, 0, 0, 0, 0, 1),
+			INODE_BASIC_SYMLINK(3),
+			't', 'g', 't',
+
+			[FRAGMENT_TABLE_OFFSET] = 0,
+			/* clang-format on */
+	};
+	mk_stub(&archive, payload, sizeof(payload));
+
+	struct SqshFile file = {0};
+	rv = sqsh__file_init(&file, &archive, 0);
+	assert(rv == 0);
+
+	struct SqshDirectoryIterator *iter =
+			sqsh_directory_iterator_new(&file, &rv);
+	assert(rv == -SQSH_ERROR_NOT_A_DIRECTORY);
+	assert(iter == NULL);
+
+	sqsh_directory_iterator_free(iter);
+	sqsh__file_cleanup(&file);
+	sqsh__archive_cleanup(&archive);
+}
+
 DECLARE_TESTS
 TEST(iter_two_files)
 TEST(iter_invalid_file_name_with_slash)
 TEST(iter_invalid_file_name_with_0)
+TEST(iter_invalid_file_type)
 END_TESTS