lib: Bail on xattr names > XATTR_NAME_MAX

Just more of me poking randomly at mkcomposefs. Yes, we clearly need to wire up a fuzzer. In this case, we have a check for xattr value length but would happily accept a key of any arbitrary length, generating a corrupted EROFS blob. `fsck.erofs` detects this. Signed-off-by: Colin Walters <[email protected]>
cgwalters · Aug 17, 2024 · 352ed23 · 352ed23
1 parent c64e5d8
commit 352ed23
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 2 deletions.
diff --git a/libcomposefs/lcfs-writer.c b/libcomposefs/lcfs-writer.c
@@ -1519,6 +1519,12 @@ int lcfs_node_set_xattr(struct lcfs_node_s *node, const char *name,
 {
 	struct lcfs_xattr_s *xattrs;
 	char *k, *v;
+
+	if (strlen(name) > XATTR_NAME_MAX) {
+		errno = ERANGE;
+		return -1;
+	}
+
 	ssize_t index = find_xattr(node, name);
 
 	if (value_len > UINT16_MAX) {

diff --git a/tests/Makefile.am b/tests/Makefile.am
@@ -4,7 +4,7 @@ TEST_ASSETS_SMALL = \
 TEST_ASSETS_SMALL_EXTRA = \
 	special.dump.version special_v1.dump.version
 
-TEST_ASSETS_SHOULD_FAIL = should-fail-long-link.dump
+TEST_ASSETS_SHOULD_FAIL = should-fail-long-link.dump should-fail-long-xattr-key.dump should-fail-long-xattr-value.dump
 TEST_ASSETS = ${TEST_ASSETS_SMALL} \
 	cs9-x86_64-developer.dump.gz cs9-x86_64-minimal.dump.gz	\
 	f36-x86_64-silverblue.dump.gz

diff --git a/tests/assets/should-fail-long-xattr-key.dump b/tests/assets/should-fail-long-xattr-key.dump
diff --git a/tests/assets/should-fail-long-xattr-value.dump b/tests/assets/should-fail-long-xattr-value.dump
diff --git a/tests/meson.build b/tests/meson.build
@@ -11,7 +11,9 @@ test_assets_small_extra = [
 ]
 
 test_assets_should_fail = [
-  'should-fail-long-link.dump'
+  'should-fail-long-link.dump',
+  'should-fail-long-xattr-key.dump',
+  'should-fail-long-xattr-value.dump',
 ]
 
 test_assets = test_assets_small + [