GNOME Bugzilla – Bug 775380
Recursive entities may defeat element limits and exhaust stack before triggering recursive entity limits
Last modified: 2021-07-05 13:22:12 UTC
This report is based on a downstream Chromium bug, <https://crbug.com/628581#c14>. Please let me know if you would like CCs on this bug to view it. An input such as this: <!DOCTYPE[ <!ENTITY a " <a> <-- ~"many" open tags &a; "> ]> <doc> &a; may overflow the stack before triggering entity recursion checks. I've done some rudimentary analysis: Entity recursion is detected through recursion counters, however an entity with many unclosed tags consumes stack much more quickly than it "consumes" the entity recursion guard counter. libxml2 has xmlParserMaxDepth, but xmlParseBalancedChunkMemoryInternal which parses the entity uses a new context for each entity expansion. Each context individually doesn't trip the xmlParserMaxDepth. I think there are two mitigations which could help here: 1. Contexts could transmit the number of open elements to their "child" contexts, and xmlParseElement could check ctxt->nodeNr + ctxt->"ancestor node count" > xmlParseMaxDepth. 2. In addition to detecting entity recursion empirically through a depth count in xmlParseBalancedChunkMemoryInternal, detect entity recursion analytically by marking entities when we start to parse them and checking that they are not already marked.
A sketch of the second idea is in this downstream patch: https://codereview.chromium.org/2539003002
I couldn't reproduce this on the latest git tree, could you publish a full reproducer, or maybe its fixed differently. Entity: line 258: parser error : Excessive depth in document: 256 use XML_PARSE_HUGE option <a> ^
I suspect what's happening here is debug builds of Chromium end up invoking libxml parsing on a stack that's already deep, so maybe this is not of interest to libxml. I tried to reproduce this with xmllint but did not succeed. I'm fine if you close this.
We managed to reproduce it once we added the --huge flag, a variation of the patch fixed the issue, i'll upload our patch
Created attachment 343343 [details] [review] Potential Patch This patch was based from 2.9.4 rather then the latest git
This patch was based off CVE-2016-9597
Splendid, thank you very much.
Couldn't it be (also) resolved by the patch proposed in bug 771799 ? The advantage (I see) there is that the user can choose (or not, compatibily) to have both --huge and exponential linear entity expansion protection at the same time.
(In reply to Simon Lees (SUSE) from comment #6) > This patch was based off CVE-2016-9597 Does this patch address CVE-2016-9597? I searched bugs related to CVE-2016-9597 for libxml2 and the result was only this defect. If this patch addresses the vulnerability, any idea when this will be resolved? OR Is there another bug that addresses CVE-2016-9597?
This is a complicated issue that I'm also working on currently. The idea of an "entity guard" is part of the solution. There are a few more places where it needs to be applied, though. But we still have to propagate the parser depth to subcontexts to fix similar issues that don't involve recursive entities. The max depth test is currently connected to the size of the "node stack". Ultimately, I'd like to use ctxt->depth for all recursion depth checks.
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new ticket at https://gitlab.gnome.org/GNOME/libxml2/-/issues/ Thank you for your understanding and your help.