lib: Verify mode values

libcomposefs/lcfs-internal.h

@@ -223,6 +223,9 @@ int node_get_dtype(struct lcfs_node_s *node);
 int lcfs_node_rename_xattr(struct lcfs_node_s *node, size_t index,
 			   const char *new_name);
 
+int lcfs_validate_mode(mode_t mode);
+int lcfs_node_last_ditch_validation(struct lcfs_node_s *node);
+
 /* lcfs-writer-erofs.c */
 
 int lcfs_write_erofs_to(struct lcfs_ctx_s *ctx);

libcomposefs/lcfs-writer-erofs.c

@@ -803,6 +803,12 @@ static int write_erofs_inode_data(struct lcfs_ctx_s *ctx, struct lcfs_node_s *no
 	uint32_t chunk_count = 0;
 	uint16_t chunk_format = 0;
 
+	// Unfortunately lcfs_node_set_mode doesn't validate today, so let's ensure we do
+	// it at this last ditch moment if we haven't managed to do it earlier.
+	if (lcfs_validate_mode(node->inode.st_mode) < 0) {
+		return -1;
+	}
+
 	ret = lcfs_write_pad(ctx, node->erofs_ipad);
 	if (ret < 0)
 		return ret;

libcomposefs/lcfs-writer.c

@@ -77,6 +77,25 @@ size_t hash_memory(const char *string, size_t len, size_t n_buckets)
 	return value % n_buckets;
 }
 
+// Verify a mode value; we don't accept unknown values
+int lcfs_validate_mode(mode_t mode)
+{
+	switch (mode & S_IFMT) {
+	case S_IFREG:
+	case S_IFDIR:
+	case S_IFLNK:
+	case S_IFBLK:
+	case S_IFCHR:
+	case S_IFSOCK:
+	case S_IFIFO:
+		return 0;
+	default: {
+		errno = EINVAL;
+		return -1;
+	}
+	}
+}
+
 static struct lcfs_ctx_s *lcfs_new_ctx(struct lcfs_node_s *root,
 				       struct lcfs_write_options_s *options)
 {
@@ -361,6 +380,12 @@ int lcfs_write_to(struct lcfs_node_s *root, struct lcfs_write_options_s *options
 	struct lcfs_ctx_s *ctx;
 	int res;
 
+	// Verify the root; because lcfs_node_add_child also verifies children,
+	// we should have sanity checked all nodes.
+	if (lcfs_node_last_ditch_validation(root) < 0) {
+		return -1;
+	}
+
 	/* Check for unknown flags */
 	if ((options->flags & ~LCFS_FLAGS_MASK) != 0) {
 		errno = EINVAL;
@@ -1078,12 +1103,30 @@ struct lcfs_node_s *lcfs_node_get_hardlink_target(struct lcfs_node_s *node)
 	return node->link_to;
 }
 
+// Unfortunately some APIs to initialize a node can create
+// invalid states, but don't return errors; this will try
+// to perform validation when the node is passed to a function
+// that does return an error.
+int lcfs_node_last_ditch_validation(struct lcfs_node_s *node)
+{
+	// Hardlinks should have mode 0
+	if (node->link_to == NULL) {
+		if (lcfs_validate_mode(node->inode.st_mode) < 0)
+			return -1;
+	}
+	return 0;
+}
+
 int lcfs_node_add_child(struct lcfs_node_s *parent, struct lcfs_node_s *child,
 			const char *name)
 {
 	struct lcfs_node_s **new_children;
 	size_t new_capacity;
 
+	if (lcfs_node_last_ditch_validation(child) < 0) {
+		return -1;
+	}
+
 	if ((parent->inode.st_mode & S_IFMT) != S_IFDIR) {
 		errno = ENOTDIR;
 		return -1;

tests/Makefile.am

@@ -4,7 +4,8 @@ TEST_ASSETS_SMALL = \
 TEST_ASSETS_SMALL_EXTRA = \
 	special.dump.version special_v1.dump.version
 
-TEST_ASSETS_SHOULD_FAIL = should-fail-long-link.dump should-fail-long-xattr-key.dump should-fail-long-xattr-value.dump
+TEST_ASSETS_SHOULD_FAIL = should-fail-long-link.dump should-fail-long-xattr-key.dump should-fail-long-xattr-value.dump \
+	should-fail-no-ftype.dump
 TEST_ASSETS = ${TEST_ASSETS_SMALL} \
 	cs9-x86_64-developer.dump.gz cs9-x86_64-minimal.dump.gz	\
 	f36-x86_64-silverblue.dump.gz

tests/assets/should-fail-no-ftype.dump

@@ -0,0 +1,2 @@
+/ 4096 40555 2 0 0 0 1633950376.0 - - - -
+/noftype 3 0 1 0 0 0 0.0 foo - -

tests/meson.build

@@ -14,6 +14,7 @@ test_assets_should_fail = [
   'should-fail-long-link.dump',
   'should-fail-long-xattr-key.dump',
   'should-fail-long-xattr-value.dump',
+  'should-fail-no-ftype.dump',
 ]
 
 test_assets = test_assets_small + [

tests/test-should-fail.sh

@@ -7,7 +7,7 @@ shift
 tmpd=$(mktemp -d -t lcfs-test.XXXXXX)
 trap 'rm -rf -- "$tmpd"' EXIT
 for f in $@; do
-    if $bindir/mkcomposefs --from-file $f $tmpd/out.cfs >/dev/null 2>err.txt; then
+    if $bindir/mkcomposefs --from-file $f $tmpd/out.cfs &>/dev/null; then
         fatal "Test case $f should have failed"
     fi
     echo "ok $f"