Make sqfs_tree_node_get_path more robust

Test against various invariants:
 - Every non-root node must have a name
 - The root node muts not have a name
 - The name must not be ".." or "."
 - The name must not contain '/'
 - The loop that chases parent pointers must terminate, i.e. we must
   never reach the starting state again (link loop).

Furthermore, make sure the sum of all path components plus separators
does not overflow.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>

10 files changed, 106 insertions, 46 deletions

diff --git a/bin/rdsquashfs/describe.c b/bin/rdsquashfs/describe.c
index 1e723bc..540b126 100644
--- a/bin/rdsquashfs/describe.c
+++ b/bin/rdsquashfs/describe.c
@@ -8,10 +8,12 @@
 
 static int print_name(const sqfs_tree_node_t *n, bool dont_escape)
 {
-	char *start, *ptr, *name = sqfs_tree_node_get_path(n);
+	char *start, *ptr, *name;
+	int ret;
 
-	if (name == NULL) {
-		perror("Recovering file path of tree node");
+	ret = sqfs_tree_node_get_path(n, &name);
+	if (ret != 0) {
+		sqfs_perror(NULL, "Recovering file path of tree node", ret);
 		return -1;
 	}
 
diff --git a/bin/rdsquashfs/fill_files.c b/bin/rdsquashfs/fill_files.c
index 8459c1e..923bc12 100644
--- a/bin/rdsquashfs/fill_files.c
+++ b/bin/rdsquashfs/fill_files.c
@@ -67,6 +67,7 @@ static int add_file(const sqfs_tree_node_t *node)
 	struct file_ent *new;
 	size_t new_sz;
 	char *path;
+	int ret;
 
 	if (num_files == max_files) {
 		new_sz = max_files ? max_files * 2 : 256;
@@ -81,9 +82,9 @@ static int add_file(const sqfs_tree_node_t *node)
 		max_files = new_sz;
 	}
 
-	path = sqfs_tree_node_get_path(node);
-	if (path == NULL) {
-		perror("assembling file path");
+	ret = sqfs_tree_node_get_path(node, &path);
+	if (ret != 0) {
+		sqfs_perror(NULL, "assembling file path", ret);
 		return -1;
 	}
 
diff --git a/bin/rdsquashfs/rdsquashfs.c b/bin/rdsquashfs/rdsquashfs.c
index a8dc4c9..8926df6 100644
--- a/bin/rdsquashfs/rdsquashfs.c
+++ b/bin/rdsquashfs/rdsquashfs.c
@@ -71,7 +71,9 @@ static int tree_sort(sqfs_tree_node_t *root)
 	for (it = root->children; it->next != NULL; it = it->next) {
 		if (strcmp((const char *)it->name,
 			   (const char *)it->next->name) == 0) {
-			char *path = sqfs_tree_node_get_path(it);
+			char *path;
+
+			sqfs_tree_node_get_path(it, &path);
 
 			fprintf(stderr, "Entry '%s' found more than once!\n",
 				path);
diff --git a/bin/rdsquashfs/restore_fstree.c b/bin/rdsquashfs/restore_fstree.c
index cf5bc2a..b9f92fe 100644
--- a/bin/rdsquashfs/restore_fstree.c
+++ b/bin/rdsquashfs/restore_fstree.c
@@ -127,10 +127,10 @@ static int create_node_dfs(const sqfs_tree_node_t *n, int flags)
 		return 0;
 	}
 
-	name = sqfs_tree_node_get_path(n);
-	if (name == NULL) {
-		fprintf(stderr, "Constructing full path for '%s': %s\n",
-			(const char *)n->name, strerror(errno));
+	ret = sqfs_tree_node_get_path(n, &name);
+	if (ret != 0) {
+		sqfs_perror((const char *)n->name,
+			    "constructing full path", ret);
 		return -1;
 	}
 
@@ -226,10 +226,9 @@ static int set_attribs(sqfs_xattr_reader_t *xattr,
 		}
 	}
 
-	path = sqfs_tree_node_get_path(n);
-	if (path == NULL) {
-		fprintf(stderr, "Reconstructing full path: %s\n",
-			strerror(errno));
+	ret = sqfs_tree_node_get_path(n, &path);
+	if (ret != 0) {
+		sqfs_perror(NULL, "reconstructing full path", ret);
 		return -1;
 	}
 
diff --git a/bin/sqfs2tar/write_tree.c b/bin/sqfs2tar/write_tree.c
index d4f8366..1c19a5a 100644
--- a/bin/sqfs2tar/write_tree.c
+++ b/bin/sqfs2tar/write_tree.c
@@ -102,9 +102,9 @@ static int write_tree_dfs(const sqfs_tree_node_t *n)
 			return 0;
 		}
 
-		name = sqfs_tree_node_get_path(n);
-		if (name == NULL) {
-			perror("resolving tree node path");
+		ret = sqfs_tree_node_get_path(n, &name);
+		if (ret != 0) {
+			sqfs_perror(NULL, "resolving tree node path", ret);
 			return -1;
 		}
 
diff --git a/bin/sqfsdiff/node_compare.c b/bin/sqfsdiff/node_compare.c
index 6e8c2fa..a0c99c7 100644
--- a/bin/sqfsdiff/node_compare.c
+++ b/bin/sqfsdiff/node_compare.c
@@ -8,13 +8,14 @@
 
 int node_compare(sqfsdiff_t *sd, sqfs_tree_node_t *a, sqfs_tree_node_t *b)
 {
-	char *path = sqfs_tree_node_get_path(a);
 	sqfs_tree_node_t *ait, *bit;
 	bool promoted, demoted;
 	int ret, status = 0;
+	char *path;
 
-	if (path == NULL) {
-		perror("constructing absolute file path");
+	ret = sqfs_tree_node_get_path(a, &path);
+	if (ret != 0) {
+		sqfs_perror(NULL, "constructing absolute file path", ret);
 		return -1;
 	}
 
diff --git a/bin/sqfsdiff/util.c b/bin/sqfsdiff/util.c
index ab6fa59..a11770f 100644
--- a/bin/sqfsdiff/util.c
+++ b/bin/sqfsdiff/util.c
@@ -8,10 +8,12 @@
 
 char *node_path(const sqfs_tree_node_t *n)
 {
-	char *path = sqfs_tree_node_get_path(n);
+	char *path;
+	int ret;
 
-	if (path == NULL) {
-		perror("get path");
+	ret = sqfs_tree_node_get_path(n, &path);
+	if (ret != 0) {
+		sqfs_perror(NULL, "get path", ret);
 		return NULL;
 	}
 
diff --git a/include/sqfs/dir_reader.h b/include/sqfs/dir_reader.h
index 3ae4ca4..e6b23f2 100644
--- a/include/sqfs/dir_reader.h
+++ b/include/sqfs/dir_reader.h
@@ -397,13 +397,23 @@ SQFS_API void sqfs_dir_tree_destroy(sqfs_tree_node_t *root);
  * non-root nodes. The resulting path is slash separated, but (except for
  * the root) never ends with a slash.
  *
+ * While walking the node list, the function enforces various invariantes. It
+ * returns @ref SQFS_ERROR_LINK_LOOP if the list of parent pointers is cyclical,
+ * @ref SQFS_ERROR_CORRUPTED if any node has an empty name, or a name that
+ * contains '/' or equals ".." or ".". The function
+ * returns @ref SQFS_ERROR_ARG_INVALID if given NULL node or the root has a name
+ * set. Additionally, the function can return overflow or allocation failures
+ * while constructing the path.
+ *
  * The returned string needs to be free'd with @ref sqfs_free.
  *
  * @param node A pointer to a tree node.
+ * @param out Returns a pointer to a string on success, set to NULL on failure.
  *
- * @return A pointer to a string on success, NULL on allocation failure.
+ * @return Zero on success, an @ref SQFS_ERROR value on failure.
  */
-SQFS_API char *sqfs_tree_node_get_path(const sqfs_tree_node_t *node);
+SQFS_API int sqfs_tree_node_get_path(const sqfs_tree_node_t *node,
+				     char **out);
 
 #ifdef __cplusplus
 }
diff --git a/lib/common/hardlink.c b/lib/common/hardlink.c
index 031724b..f8d4d4a 100644
--- a/lib/common/hardlink.c
+++ b/lib/common/hardlink.c
@@ -49,9 +49,9 @@ static int map_nodes(rbtree_t *inumtree, sqfs_hard_link_t **out,
 		goto fail_oom;
 
 	lnk->inode_number = idx;
-	lnk->target = sqfs_tree_node_get_path(target);
-	if (lnk->target == NULL)
-		goto fail_oom;
+	ret = sqfs_tree_node_get_path(target, &lnk->target);
+	if (ret != 0)
+		goto fail_path;
 
 	if (canonicalize_name(lnk->target) == 0) {
 		lnk->next = (*out);
@@ -61,6 +61,10 @@ static int map_nodes(rbtree_t *inumtree, sqfs_hard_link_t **out,
 		free(lnk);
 	}
 	return 0;
+fail_path:
+	sqfs_perror(NULL, "re-constructing hard link path", ret);
+	free(lnk);
+	return -1;
 fail_oom:
 	fputs("detecting hard links in file system tree: out of memory\n",
 	      stderr);
diff --git a/lib/sqfs/dir_reader/get_path.c b/lib/sqfs/dir_reader/get_path.c
index bdd8317..847bfd3 100644
--- a/lib/sqfs/dir_reader/get_path.c
+++ b/lib/sqfs/dir_reader/get_path.c
@@ -10,33 +10,72 @@
 #include <string.h>
 #include <stdlib.h>
 
-char *sqfs_tree_node_get_path(const sqfs_tree_node_t *node)
+int sqfs_tree_node_get_path(const sqfs_tree_node_t *node, char **out)
 {
 	const sqfs_tree_node_t *it;
+	size_t clen, len = 0;
 	char *str, *ptr;
-	size_t len = 0;
 
-	if (node->parent == NULL)
-		return strdup("/");
+	*out = NULL;
 
-	for (it = node; it != NULL && it->parent != NULL; it = it->parent) {
-		len += strlen((const char *)it->name) + 1;
+	if (node == NULL)
+		return SQFS_ERROR_ARG_INVALID;
+
+	for (it = node; it->parent != NULL; it = it->parent) {
+		if (it->parent == node)
+			return SQFS_ERROR_LINK_LOOP;
+
+		/* non-root nodes must have a valid name */
+		clen = strlen((const char *)it->name);
+
+		if (clen == 0)
+			return SQFS_ERROR_CORRUPTED;
+
+		if (strchr((const char *)it->name, '/') != NULL)
+			return SQFS_ERROR_CORRUPTED;
+
+		if (it->name[0] == '.') {
+			if (clen == 1 || (clen == 2 && it->name[1] == '.'))
+				return SQFS_ERROR_CORRUPTED;
+		}
+
+		/* compute total path length */
+		if (SZ_ADD_OV(clen, 1, &clen))
+			return SQFS_ERROR_OVERFLOW;
+
+		if (SZ_ADD_OV(len, clen, &len))
+			return SQFS_ERROR_OVERFLOW;
 	}
 
-	str = malloc(len + 1);
-	if (str == NULL)
-		return NULL;
+	/* root node must not have a name */
+	if (it->name[0] != '\0')
+		return SQFS_ERROR_ARG_INVALID;
+
+	/* generate the path */
+	if (node->parent == NULL) {
+		str = strdup("/");
+		if (str == NULL)
+			return SQFS_ERROR_ALLOC;
+	} else {
+		if (SZ_ADD_OV(len, 1, &len))
+			return SQFS_ERROR_OVERFLOW;
+
+		str = malloc(len);
+		if (str == NULL)
+			return SQFS_ERROR_ALLOC;
 
-	ptr = str + len;
-	*ptr = '\0';
+		ptr = str + len - 1;
+		*ptr = '\0';
 
-	for (it = node; it != NULL && it->parent != NULL; it = it->parent) {
-		len = strlen((const char *)it->name);
-		ptr -= len;
+		for (it = node; it->parent != NULL; it = it->parent) {
+			len = strlen((const char *)it->name);
+			ptr -= len;
 
-		memcpy(ptr, (const char *)it->name, len);
-		*(--ptr) = '/';
+			memcpy(ptr, (const char *)it->name, len);
+			*(--ptr) = '/';
+		}
 	}
 
-	return str;
+	*out = str;
+	return 0;
 }