Harden index XML parsing against malformed input#614
Open
matejk wants to merge 2 commits into
Open
Conversation
_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 <directory> 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).
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.
matejk
force-pushed
the
fix/index-parsing-hardening
branch
from
d031cd7 to
9293668
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Defects reachable from a crafted or truncated index while mounting a cartridge. Two commits:
_xml_parse_dirtreerecurses once per directory level and the index reader setsXML_PARSE_HUGE, which removes libxml2's own nesting limit, so deeply nested<directory>elements could overflow the C stack during mount. The bound (5051) is a stack-safety guard, not an LTFS format limit — the format defines no maximum depth — and is set well above any tree that fits in a conventionalPATH_MAX.xml_next_tagpassed a possibly-NULL libxml2 node name tostrcmp;decode_entry_namewrote to an allocation of attacker-controlled size without a NULL check; theglob_patternsgrowth dereferenced an uncheckedrealloc; anltfs.vendor.IBM.immutable/appendonlyxattr with an empty self-closing value passed NULL tostrcmpwhen deciding WORM flags, crashing the mount.