Fix unrestricted entity expansion in ezxml_decode (billion laughs) - #11
Fix unrestricted entity expansion in ezxml_decode (billion laughs)#11esadowski4 wants to merge 1 commit into
Conversation
Add EZXML_MAX_ENTITY_EXPANSION limit (8 MB) to prevent exponential memory growth from nested entity definitions. ezxml_decode() expands entity references by substituting their values into the working string, then continues scanning forward through the substituted text. Entity references within the replacement are expanded in turn. ezxml_ent_ok() only rejects direct circular references (A -> B -> A), not exponential expansion through fan-out (A -> B+B, B -> C+C, ...). An XML document with 30 levels of nested entity definitions: <!ENTITY x0 "BOOM"> <!ENTITY x1 "&x0;&x0;"> ... <!ENTITY x30 "&x29;&x29;"> produces 4 GB of expanded text from ~700 bytes of input (177,302x amplification). The process hangs consuming memory until killed. The fix tracks cumulative bytes of entity replacement text in ezxml_decode(). When total expansion exceeds 8 MB, the loop breaks. 8 MB is far above what any legitimate XML document needs. Also changed the size calculation variable from long to size_t to prevent signed integer overflow when computing buffer sizes during entity expansion.
There was a problem hiding this comment.
Pull request overview
This PR mitigates a high-severity DoS vulnerability in ezxml_decode() caused by unrestricted/nested XML entity expansion (“billion laughs”), by introducing a hard cap on total entity-expanded bytes during decoding and tightening length arithmetic types.
Changes:
- Added
EZXML_MAX_ENTITY_EXPANSION(8 MB) and atotal_expandedcounter to stop excessive entity expansion. - Switched the allocation-length variable
lfromlongtosize_tand replaced the pointer-offset temporary withsize_tfor safer length/offset arithmetic. - Minor cast adjustment for
strspn()usage withlnow beingsize_t.
Comments suppressed due to low confidence (1)
ezxml.c:214
cis reused for both character-codepoint decoding and entity replacement length (strlen(ent[b])), which forces awkward signed/unsigned conversions (s + c,strncpy(..., c),(size_t)cfortotal_expanded). To avoid truncation risk and make the new size/limit logic clearer, consider introducing a separatesize_t rep_len = strlen(ent[b]);for entity expansion and keepcfor numeric character references only.
c = strlen(ent[b]);
total_expanded += (size_t)c;
if (total_expanded > EZXML_MAX_ENTITY_EXPANSION)
break; // entity expansion limit exceeded
if (c - 1 > (e = strchr(s, ';')) - s) {
size_t d_off = (size_t)(s - r);
l = d_off + (size_t)c + strlen(e); // new length
r = (r == m) ? strcpy(malloc(l), r) : realloc(r, l);
e = strchr((s = r + d_off), ';'); // fix up pointers
}
memmove(s + c, e + 1, strlen(e)); // shift rest of string
strncpy(s, ent[b], c); // copy in replacement text
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| c = strlen(ent[b]); | ||
| total_expanded += (size_t)c; | ||
| if (total_expanded > EZXML_MAX_ENTITY_EXPANSION) | ||
| break; // entity expansion limit exceeded | ||
|
|
There was a problem hiding this comment.
The break on entity-expansion limit exits the main decode loop entirely, which also stops decoding/normalization for the remainder of the string (e.g., later character references and whitespace normalization for t == ' ' / t == '*'). Consider handling the limit by skipping further entity expansion while continuing to scan/normalize the rest of the buffer (e.g., treat the entity as unknown and advance s, or use a flag to disable expansion) so non-entity decoding remains consistent even when the limit is hit.
Summary
Vulnerability class: Denial of service - unrestricted XML entity expansion (billion laughs / XML bomb)
Severity: High (DoS)
Affected file and line range:
ezxml.c,ezxml_decode()(line 158), specifically the entity reference expansion loop at lines 194-203Description
ezxml_decode()expands entity references by substituting their values into the working string, then continues scanning forward through the substituted text. If the replacement contains further entity references, those are expanded in turn.ezxml_ent_ok()only rejects direct circular references (A references B references A) - it does not detect or limit exponential expansion through fan-out.An XML document with nested entity definitions exploits this:
Each level doubles the output. With 30 levels, ~700 bytes of input expands to 4 GB of text. The process hangs consuming memory until killed.
Attack vector
Any application using ezxml to parse untrusted XML input. The attacker provides a small XML document with nested entity definitions. The parser expands entities recursively with no bound on total expansion size.
Proof of concept
Compile:
gcc -o poc poc.c ezxml.c -I.Tested results on this codebase:
Patch explanation
Two changes to
ezxml_decode()inezxml.c:Add
EZXML_MAX_ENTITY_EXPANSIONlimit (8 MB) - atotal_expandedcounter accumulates the byte length of every entity replacement. When it exceeds 8 MB, the expansion loop breaks. 8 MB is far above what any legitimate XML document needs. This directly prevents the exponential blowup.Change size variable
lfromlongtosize_t- on platforms wherelongis 32 bits, the size calculationl = (d = (s - r)) + c + strlen(e)can overflow when the working string is large. Usingsize_tmatches the width of pointer arithmetic and allocation sizes. A localsize_t d_offreplaces thelong dfor the same reason.Normal entity expansion (
&,<, custom entities in small documents) is unaffected - the 8 MB limit is only reached by pathological inputs.The existing
ezxml_ent_ok()guard only detects direct circular entity references (entity A references entity B, which references entity A). It does not detect exponential expansion where each entity references two copies of another entity. This is the same class of vulnerability as CVE-2003-1564 (libxml2) and CVE-2013-1664 (Python xml.etree).