From 4d885892dde6f7ea87e1ade32ced2567bc9f74be Mon Sep 17 00:00:00 2001 From: Enno Boland Date: Tue, 1 Aug 2023 19:00:00 +0200 Subject: [PATCH] walker.c: fix segfault when calling _down() on an uninitialized walker. --- include/sqsh_error.h | 1 + lib/tree/walker.c | 4 +- lib/utils/error.c | 2 + test/tree/walker.c | 116 ++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 121 insertions(+), 2 deletions(-) diff --git a/include/sqsh_error.h b/include/sqsh_error.h index f7f895d7e..fb0e0eb0b 100644 --- a/include/sqsh_error.h +++ b/include/sqsh_error.h @@ -80,6 +80,7 @@ enum SqshError { SQSL_ERROR_ELEMENT_NOT_FOUND, SQSH_ERROR_INVALID_ARGUMENT, SQSH_ERROR_WALKER_CANNOT_GO_UP, + SQSH_ERROR_WALKER_CANNOT_GO_DOWN, SQSH_ERROR_CORRUPTED_INODE, SQSH_ERROR_INTERNAL, SQSH_ERROR_INODE_MAP_IS_INCONSISTENT, diff --git a/lib/tree/walker.c b/lib/tree/walker.c index ba30a0bd4..e0cb1a5d5 100644 --- a/lib/tree/walker.c +++ b/lib/tree/walker.c @@ -210,7 +210,6 @@ sqsh_tree_walker_revert(struct SqshTreeWalker *walker) { struct SqshInode *inode = &walker->directory; struct SqshDirectoryIterator *iterator = &walker->iterator; - walker->begin_iterator = true; sqsh__directory_iterator_cleanup(iterator); walker->begin_iterator = true; return sqsh__directory_iterator_init(iterator, inode); @@ -237,6 +236,9 @@ sqsh_tree_walker_lookup( int sqsh_tree_walker_down(struct SqshTreeWalker *walker) { + if (walker->begin_iterator == true) { + return -SQSH_ERROR_WALKER_CANNOT_GO_DOWN; + } const uint64_t child_inode_ref = sqsh_directory_iterator_inode_ref(&walker->iterator); diff --git a/lib/utils/error.c b/lib/utils/error.c index c7c1ea82a..a27312adf 100644 --- a/lib/utils/error.c +++ b/lib/utils/error.c @@ -122,6 +122,8 @@ sqsh_error_str(int error_code) { return "Invalid argument"; case SQSH_ERROR_WALKER_CANNOT_GO_UP: return "Walker cannot go up"; + case SQSH_ERROR_WALKER_CANNOT_GO_DOWN: + return "Walker cannot go down"; case SQSH_ERROR_CORRUPTED_INODE: return "Corrupted inode"; case SQSH_ERROR_INTERNAL: diff --git a/test/tree/walker.c b/test/tree/walker.c index 549ec431e..895639f2f 100644 --- a/test/tree/walker.c +++ b/test/tree/walker.c @@ -53,7 +53,7 @@ walker_symlink_open(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', @@ -82,6 +82,120 @@ walker_symlink_open(void) { sqsh__archive_cleanup(&archive); } +static void +walker_directory_enter(void) { + int rv; + struct SqshArchive archive = {0}; + struct SqshInode *inode = NULL; + uint8_t payload[] = { + /* clang-format off */ + SQSH_HEADER, + /* inode */ + [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_HEADER(1, 0, 0, 0, 0, 2), + INODE_BASIC_DIR(256, 1024, 0, 0), + + [DIRECTORY_TABLE_OFFSET] = METABLOCK_HEADER(0, 128), + DIRECTORY_HEADER(1, 0, 0), + DIRECTORY_ENTRY(128, 2, 1, 3), + 'd', 'i', 'r', + + [DIRECTORY_TABLE_OFFSET+256] = METABLOCK_HEADER(0, 128), + DIRECTORY_HEADER(0, 0, 0), + + [FRAGMENT_TABLE_OFFSET] = 0, + /* clang-format on */ + }; + mk_stub(&archive, payload, sizeof(payload)); + + struct SqshTreeWalker walker = {0}; + rv = sqsh__tree_walker_init(&walker, &archive); + assert(rv == 0); + + rv = sqsh_tree_walker_resolve(&walker, "dir", true); + assert(rv == 0); + + inode = sqsh_tree_walker_inode_load(&walker, &rv); + assert(rv == 0); + assert(inode != NULL); + assert(sqsh_inode_number(inode) == 2); + + rv = sqsh_tree_walker_up(&walker); + assert(rv == 0); + + sqsh__tree_walker_cleanup(&walker); + sqsh__archive_cleanup(&archive); +} + +static void +walker_uninitialized_up(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(1, 0, 0, 0, 0, 1), + INODE_BASIC_DIR(0, 1024, 0, 0), + + [DIRECTORY_TABLE_OFFSET] = METABLOCK_HEADER(0, 128), + DIRECTORY_HEADER(0, 0, 0), + + [FRAGMENT_TABLE_OFFSET] = 0, + /* clang-format on */ + }; + mk_stub(&archive, payload, sizeof(payload)); + + struct SqshTreeWalker walker = {0}; + rv = sqsh__tree_walker_init(&walker, &archive); + assert(rv == 0); + + rv = sqsh_tree_walker_up(&walker); + assert(rv == -SQSH_ERROR_WALKER_CANNOT_GO_UP); + + sqsh__tree_walker_cleanup(&walker); + sqsh__archive_cleanup(&archive); +} + +static void +walker_uninitialized_down(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(1, 0, 0, 0, 0, 1), + INODE_BASIC_DIR(0, 1024, 0, 0), + + [DIRECTORY_TABLE_OFFSET] = METABLOCK_HEADER(0, 128), + DIRECTORY_HEADER(0, 0, 0), + + [FRAGMENT_TABLE_OFFSET] = 0, + /* clang-format on */ + }; + mk_stub(&archive, payload, sizeof(payload)); + + struct SqshTreeWalker walker = {0}; + rv = sqsh__tree_walker_init(&walker, &archive); + assert(rv == 0); + + rv = sqsh_tree_walker_down(&walker); + assert(rv == -SQSH_ERROR_WALKER_CANNOT_GO_DOWN); + + sqsh__tree_walker_cleanup(&walker); + sqsh__archive_cleanup(&archive); +} + DECLARE_TESTS TEST(walker_symlink_open) +NO_TEST(walker_directory_enter) +TEST(walker_uninitialized_down) +TEST(walker_uninitialized_up) END_TESTS