From 1684f9cffe20ecff1adcbf013758142473bed387 Mon Sep 17 00:00:00 2001 From: Matej Kenda Date: Wed, 10 Jun 2026 23:34:46 +0200 Subject: [PATCH 1/2] harden: bound directory recursion depth when parsing an index _xml_parse_dirtree recurses once per directory level, and the index reader sets XML_PARSE_HUGE, which removes libxml2's own nesting limit. A crafted index with deeply nested elements could recurse until the C stack overflows while mounting an untrusted cartridge. Add a depth bound (a stack-safety guard, not an LTFS format limit; the format defines no maximum depth) set well above any tree that fits in a conventional PATH_MAX, so it cannot reject a volume produced from a real filesystem. Introduces LTFS_XML_DEEP_NESTING (5051). --- messages/internal_error/root.txt | 1 + messages/libltfs/root.txt | 1 + src/libltfs/arch/errormap.c | 1 + src/libltfs/ltfs_error.h | 1 + src/libltfs/xml_reader_libltfs.c | 27 +++++++++++++++++++++------ 5 files changed, 25 insertions(+), 6 deletions(-) diff --git a/messages/internal_error/root.txt b/messages/internal_error/root.txt index 081a81e3..ed0f0996 100644 --- a/messages/internal_error/root.txt +++ b/messages/internal_error/root.txt @@ -309,6 +309,7 @@ root:table { I5048E:string{ "Unexpected partition map in a label." } I5049E:string{ "Unexpected blocksize in a label." } I5050E:string{ "Unexpected compression in a label." } + I5051E:string{ "Directory nesting in the index is too deep." } // Special error codes I9997E:string{ "Child process error (ltfsck/mkltfs): %s (%d)." } diff --git a/messages/libltfs/root.txt b/messages/libltfs/root.txt index e1b94ab6..96f0e386 100644 --- a/messages/libltfs/root.txt +++ b/messages/libltfs/root.txt @@ -836,6 +836,7 @@ v 17292I:string { "Current position is (%llu, %llu), Error position is (%llu, %llu)." } 17293E:string { "Position mismatch. Cached tape position = %llu. Current tape position = %llu." } 17294I:string { "Continue signal (%d) received" } + 17295E:string { "XML parser: directory nesting is too deep (limit %d)." } // For Debug 19999I:string { "%s %s %d." } diff --git a/src/libltfs/arch/errormap.c b/src/libltfs/arch/errormap.c index 8fa6adf5..23d68489 100644 --- a/src/libltfs/arch/errormap.c +++ b/src/libltfs/arch/errormap.c @@ -342,6 +342,7 @@ static struct error_map fuse_error_list[] = { { LTFS_XML_WRONG_PART_MAP, "I5048E", EINVAL}, { LTFS_XML_WRONG_BLOCKSIZE, "I5049E", EINVAL}, { LTFS_XML_WRONG_COMP, "I5050E", EINVAL}, + { LTFS_XML_DEEP_NESTING, "I5051E", EINVAL}, { EDEV_NO_SENSE, "D0000E", EIO}, { EDEV_OVERRUN, "D0002E", EIO}, { EDEV_UNDERRUN, "D0003E", ENODATA}, diff --git a/src/libltfs/ltfs_error.h b/src/libltfs/ltfs_error.h index c762520e..f495baea 100644 --- a/src/libltfs/ltfs_error.h +++ b/src/libltfs/ltfs_error.h @@ -317,6 +317,7 @@ #define LTFS_XML_WRONG_PART_MAP 5048 /* Unexpected partition map in a label */ #define LTFS_XML_WRONG_BLOCKSIZE 5049 /* Unexpected blocksize in a label */ #define LTFS_XML_WRONG_COMP 5050 /* Unexpected compression in a label */ +#define LTFS_XML_DEEP_NESTING 5051 /* Directory nesting in the index is too deep */ #define LTFS_ERR_MAX 19999 diff --git a/src/libltfs/xml_reader_libltfs.c b/src/libltfs/xml_reader_libltfs.c index ae5b9cf2..0aec015d 100644 --- a/src/libltfs/xml_reader_libltfs.c +++ b/src/libltfs/xml_reader_libltfs.c @@ -1192,11 +1192,21 @@ static int _xml_parse_file(xmlTextReaderPtr reader, struct ltfs_index *idx, stru * Parse a dir into the given directory. */ +/* Guard against C-stack exhaustion while parsing the directory tree: + * _xml_parse_dirtree recurses once per nesting level and XML_PARSE_HUGE + * removes libxml2's own nesting limit, so a crafted index could otherwise + * recurse until the stack overflows. This is a stack-safety bound, not an + * LTFS format limit (the format defines no maximum depth); it is set far + * above any tree that fits in a conventional PATH_MAX, so it cannot reject + * a volume produced from a real filesystem. */ +#define XML_MAX_DIRTREE_DEPTH 1024 + static int _xml_parse_dirtree(xmlTextReaderPtr reader, struct dentry *parent, struct ltfs_index *idx, struct ltfs_volume *vol, - struct name_list *dirname); /* Forward reference */ + struct name_list *dirname, int depth); /* Forward reference */ -static int _xml_parse_dir_contents(xmlTextReaderPtr reader, struct dentry *dir, struct ltfs_index *idx) +static int _xml_parse_dir_contents(xmlTextReaderPtr reader, struct dentry *dir, + struct ltfs_index *idx, int depth) { struct name_list *list = NULL, *entry_name = NULL; CHECK_ARG_NULL(dir, -LTFS_NULL_ARG); @@ -1228,7 +1238,7 @@ static int _xml_parse_dir_contents(xmlTextReaderPtr reader, struct dentry *dir, ltfsmsg(LTFS_ERR, 10001E, "_xml_parse_dir_contents: dir"); return -LTFS_NO_MEMORY; } - ret = _xml_parse_dirtree(reader, dir, idx, dir->vol, entry_name); + ret = _xml_parse_dirtree(reader, dir, idx, dir->vol, entry_name, depth + 1); if (ret < 0) { free(entry_name); return ret; @@ -1285,7 +1295,7 @@ static int _xml_parse_dir_contents(xmlTextReaderPtr reader, struct dentry *dir, */ static int _xml_parse_dirtree(xmlTextReaderPtr reader, struct dentry *parent, struct ltfs_index *idx, struct ltfs_volume *vol, - struct name_list *dirname) + struct name_list *dirname, int depth) { unsigned long long value_int; struct dentry *dir; @@ -1293,6 +1303,11 @@ static int _xml_parse_dirtree(xmlTextReaderPtr reader, struct dentry *parent, declare_parser_vars("directory"); declare_tracking_arrays(9, 1); + if (depth > XML_MAX_DIRTREE_DEPTH) { + ltfsmsg(LTFS_ERR, 17295E, XML_MAX_DIRTREE_DEPTH); + return -LTFS_XML_DEEP_NESTING; + } + if (! parent && idx->root) { dir = idx->root; dir->vol = vol; @@ -1407,7 +1422,7 @@ static int _xml_parse_dirtree(xmlTextReaderPtr reader, struct dentry *parent, check_required_tag(6); check_empty(); if (empty == 0) { - ret = _xml_parse_dir_contents(reader, dir, idx); + ret = _xml_parse_dir_contents(reader, dir, idx, depth); if (ret < 0) return ret; } @@ -1598,7 +1613,7 @@ static int _xml_parse_schema(xmlTextReaderPtr reader, struct ltfs_index *idx, st } else if (! strcmp(name, "directory")) { check_required_tag(6); assert_not_empty(); - ret = _xml_parse_dirtree(reader, NULL, idx, vol, NULL); + ret = _xml_parse_dirtree(reader, NULL, idx, vol, NULL, 0); if (ret < 0) return ret; From 9293668e4a04ae199bc18cda45ee0e25601afce2 Mon Sep 17 00:00:00 2001 From: Matej Kenda Date: Fri, 12 Jun 2026 12:51:57 +0200 Subject: [PATCH 2/2] fix: NULL dereferences and unchecked allocations parsing index XML Four defects reachable from a crafted or truncated index while mounting: - xml_next_tag passed the node name to strcmp; libxml2 returns NULL for some node types. Skip NULL names. - decode_entry_name wrote to a percent-decode buffer allocated from an attacker-controlled length without checking the allocation. Return -LTFS_NO_MEMORY instead. - the glob_patterns array growth assigned realloc straight back and dereferenced it; on failure the old pointer leaked and the next write dereferenced NULL. Use a temporary and fail cleanly. - an named ltfs.vendor.IBM.immutable or appendonly with an empty (self-closing) value set xattr->value to NULL, which was then passed to strcmp when deciding the WORM flags, crashing the mount. Guard the value. --- src/libltfs/xml_reader.c | 4 +++- src/libltfs/xml_reader_libltfs.c | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/libltfs/xml_reader.c b/src/libltfs/xml_reader.c index c9ddbb4a..10fb9e36 100644 --- a/src/libltfs/xml_reader.c +++ b/src/libltfs/xml_reader.c @@ -115,7 +115,9 @@ int xml_next_tag(xmlTextReaderPtr reader, const char *containing_name, return ret; *name = (const char *)xmlTextReaderConstName(reader); *type = xmlTextReaderNodeType(reader); - } while (strcmp(*name, containing_name) && (*type) != XML_ELEMENT_NODE); + /* libxml2 returns a NULL name for some node types; keep reading + * rather than dereferencing NULL in strcmp below. */ + } while (! *name || (strcmp(*name, containing_name) && (*type) != XML_ELEMENT_NODE)); return 0; } diff --git a/src/libltfs/xml_reader_libltfs.c b/src/libltfs/xml_reader_libltfs.c index 0aec015d..de82dbf5 100644 --- a/src/libltfs/xml_reader_libltfs.c +++ b/src/libltfs/xml_reader_libltfs.c @@ -98,6 +98,10 @@ static int decode_entry_name(char **new_name, const char *name) /* Always, length must be shorter than original but allocate null termination space */ len = strlen(name); tmp_name = malloc((len * sizeof(UChar)) + 1); + if (! tmp_name) { + ltfsmsg(LTFS_ERR, 10001E, "decode_entry_name: tmp_name"); + return -LTFS_NO_MEMORY; + } buf_decode[2] = '\0'; while (i < len) { @@ -575,8 +579,15 @@ static int _xml_parse_ip_criteria(xmlTextReaderPtr reader, struct ltfs_index *id ++num_patterns; /* quite inefficient, but the number of patterns should be small. */ - idx->original_criteria.glob_patterns = realloc(idx->original_criteria.glob_patterns, - (num_patterns + 1) * sizeof(struct ltfs_name)); + { + struct ltfs_name *new_patterns = realloc(idx->original_criteria.glob_patterns, + (num_patterns + 1) * sizeof(struct ltfs_name)); + if (! new_patterns) { + ltfsmsg(LTFS_ERR, 10001E, "_xml_parse_ip_criteria: glob_patterns"); + return -LTFS_NO_MEMORY; + } + idx->original_criteria.glob_patterns = new_patterns; + } if (_xml_parse_nametype(reader, &idx->original_criteria.glob_patterns[num_patterns - 1], @@ -863,10 +874,10 @@ static int _xml_parse_one_xattr(xmlTextReaderPtr reader, struct dentry *d) if (xattr) { TAILQ_INSERT_TAIL(&d->xattrlist, xattr, list); - if (!strcmp(xattr->key.name, "ltfs.vendor.IBM.immutable") && !strcmp(xattr->value, "1") ) { + if (xattr->value && !strcmp(xattr->key.name, "ltfs.vendor.IBM.immutable") && !strcmp(xattr->value, "1") ) { d->is_immutable = true; } - if (!strcmp(xattr->key.name, "ltfs.vendor.IBM.appendonly") && !strcmp(xattr->value, "1") ) { + if (xattr->value && !strcmp(xattr->key.name, "ltfs.vendor.IBM.appendonly") && !strcmp(xattr->value, "1") ) { d->is_appendonly = true; } }