Skip to content

Commit 9293668

Browse files
committed
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 <xattr> 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.
1 parent 1684f9c commit 9293668

2 files changed

Lines changed: 18 additions & 5 deletions

File tree

src/libltfs/xml_reader.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,9 @@ int xml_next_tag(xmlTextReaderPtr reader, const char *containing_name,
115115
return ret;
116116
*name = (const char *)xmlTextReaderConstName(reader);
117117
*type = xmlTextReaderNodeType(reader);
118-
} while (strcmp(*name, containing_name) && (*type) != XML_ELEMENT_NODE);
118+
/* libxml2 returns a NULL name for some node types; keep reading
119+
* rather than dereferencing NULL in strcmp below. */
120+
} while (! *name || (strcmp(*name, containing_name) && (*type) != XML_ELEMENT_NODE));
119121

120122
return 0;
121123
}

src/libltfs/xml_reader_libltfs.c

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ static int decode_entry_name(char **new_name, const char *name)
9898
/* Always, length must be shorter than original but allocate null termination space */
9999
len = strlen(name);
100100
tmp_name = malloc((len * sizeof(UChar)) + 1);
101+
if (! tmp_name) {
102+
ltfsmsg(LTFS_ERR, 10001E, "decode_entry_name: tmp_name");
103+
return -LTFS_NO_MEMORY;
104+
}
101105
buf_decode[2] = '\0';
102106

103107
while (i < len) {
@@ -575,8 +579,15 @@ static int _xml_parse_ip_criteria(xmlTextReaderPtr reader, struct ltfs_index *id
575579

576580
++num_patterns;
577581
/* quite inefficient, but the number of patterns should be small. */
578-
idx->original_criteria.glob_patterns = realloc(idx->original_criteria.glob_patterns,
579-
(num_patterns + 1) * sizeof(struct ltfs_name));
582+
{
583+
struct ltfs_name *new_patterns = realloc(idx->original_criteria.glob_patterns,
584+
(num_patterns + 1) * sizeof(struct ltfs_name));
585+
if (! new_patterns) {
586+
ltfsmsg(LTFS_ERR, 10001E, "_xml_parse_ip_criteria: glob_patterns");
587+
return -LTFS_NO_MEMORY;
588+
}
589+
idx->original_criteria.glob_patterns = new_patterns;
590+
}
580591

581592
if (_xml_parse_nametype(reader,
582593
&idx->original_criteria.glob_patterns[num_patterns - 1],
@@ -863,10 +874,10 @@ static int _xml_parse_one_xattr(xmlTextReaderPtr reader, struct dentry *d)
863874
if (xattr) {
864875
TAILQ_INSERT_TAIL(&d->xattrlist, xattr, list);
865876

866-
if (!strcmp(xattr->key.name, "ltfs.vendor.IBM.immutable") && !strcmp(xattr->value, "1") ) {
877+
if (xattr->value && !strcmp(xattr->key.name, "ltfs.vendor.IBM.immutable") && !strcmp(xattr->value, "1") ) {
867878
d->is_immutable = true;
868879
}
869-
if (!strcmp(xattr->key.name, "ltfs.vendor.IBM.appendonly") && !strcmp(xattr->value, "1") ) {
880+
if (xattr->value && !strcmp(xattr->key.name, "ltfs.vendor.IBM.appendonly") && !strcmp(xattr->value, "1") ) {
870881
d->is_appendonly = true;
871882
}
872883
}

0 commit comments

Comments
 (0)