Handle more invalid entity values in recovery mode
In attribute content, don't emit entity references if there are
problems with the entity value. Otherwise some illegal entity values
like
<!ENTITY a '&#x123456789;'>
would later cause problems like integer overflow.
Make xmlStringLenDecodeEntities return NULL on more error conditions
including invalid char refs and errors from recursive calls. Remove
some fragile error checks based on lastError that shouldn't be
needed now. Clear the entity content in xmlParseAttValueComplex if
an error was found.
Found by OSS-Fuzz. Should fix bug 783052.
Also see https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=3343
diff --git a/parser.c b/parser.c
index 6c40b2e..770f846 100644
--- a/parser.c
+++ b/parser.c
@@ -2636,9 +2636,9 @@
if (c == 0) break;
if ((c == '&') && (str[1] == '#')) {
int val = xmlParseStringCharRef(ctxt, &str);
- if (val != 0) {
- COPY_BUF(0,buffer,nbchars,val);
- }
+ if (val == 0)
+ goto int_error;
+ COPY_BUF(0,buffer,nbchars,val);
if (nbchars + XML_PARSER_BUFFER_SIZE > buffer_size) {
growBuffer(buffer, XML_PARSER_BUFFER_SIZE);
}
@@ -2648,9 +2648,6 @@
"String decoding Entity Reference: %.30s\n",
str);
ent = xmlParseStringEntityRef(ctxt, &str);
- if ((ctxt->lastError.code == XML_ERR_ENTITY_LOOP) ||
- (ctxt->lastError.code == XML_ERR_INTERNAL_ERROR))
- goto int_error;
xmlParserEntityCheck(ctxt, 0, ent, 0);
if (ent != NULL)
ctxt->nbentities += ent->checked / 2;
@@ -2664,30 +2661,27 @@
} else {
xmlFatalErrMsg(ctxt, XML_ERR_INTERNAL_ERROR,
"predefined entity has no content\n");
+ goto int_error;
}
} else if ((ent != NULL) && (ent->content != NULL)) {
ctxt->depth++;
rep = xmlStringDecodeEntities(ctxt, ent->content, what,
0, 0, 0);
ctxt->depth--;
+ if (rep == NULL)
+ goto int_error;
- if ((ctxt->lastError.code == XML_ERR_ENTITY_LOOP) ||
- (ctxt->lastError.code == XML_ERR_INTERNAL_ERROR))
- goto int_error;
-
- if (rep != NULL) {
- current = rep;
- while (*current != 0) { /* non input consuming loop */
- buffer[nbchars++] = *current++;
- if (nbchars + XML_PARSER_BUFFER_SIZE > buffer_size) {
- if (xmlParserEntityCheck(ctxt, nbchars, ent, 0))
- goto int_error;
- growBuffer(buffer, XML_PARSER_BUFFER_SIZE);
- }
- }
- xmlFree(rep);
- rep = NULL;
- }
+ current = rep;
+ while (*current != 0) { /* non input consuming loop */
+ buffer[nbchars++] = *current++;
+ if (nbchars + XML_PARSER_BUFFER_SIZE > buffer_size) {
+ if (xmlParserEntityCheck(ctxt, nbchars, ent, 0))
+ goto int_error;
+ growBuffer(buffer, XML_PARSER_BUFFER_SIZE);
+ }
+ }
+ xmlFree(rep);
+ rep = NULL;
} else if (ent != NULL) {
int i = xmlStrlen(ent->name);
const xmlChar *cur = ent->name;
@@ -2705,8 +2699,6 @@
xmlGenericError(xmlGenericErrorContext,
"String decoding PE Reference: %.30s\n", str);
ent = xmlParseStringPEReference(ctxt, &str);
- if (ctxt->lastError.code == XML_ERR_ENTITY_LOOP)
- goto int_error;
xmlParserEntityCheck(ctxt, 0, ent, 0);
if (ent != NULL)
ctxt->nbentities += ent->checked / 2;
@@ -2732,19 +2724,19 @@
rep = xmlStringDecodeEntities(ctxt, ent->content, what,
0, 0, 0);
ctxt->depth--;
- if (rep != NULL) {
- current = rep;
- while (*current != 0) { /* non input consuming loop */
- buffer[nbchars++] = *current++;
- if (nbchars + XML_PARSER_BUFFER_SIZE > buffer_size) {
- if (xmlParserEntityCheck(ctxt, nbchars, ent, 0))
- goto int_error;
- growBuffer(buffer, XML_PARSER_BUFFER_SIZE);
- }
- }
- xmlFree(rep);
- rep = NULL;
- }
+ if (rep == NULL)
+ goto int_error;
+ current = rep;
+ while (*current != 0) { /* non input consuming loop */
+ buffer[nbchars++] = *current++;
+ if (nbchars + XML_PARSER_BUFFER_SIZE > buffer_size) {
+ if (xmlParserEntityCheck(ctxt, nbchars, ent, 0))
+ goto int_error;
+ growBuffer(buffer, XML_PARSER_BUFFER_SIZE);
+ }
+ }
+ xmlFree(rep);
+ rep = NULL;
}
} else {
COPY_BUF(l,buffer,nbchars,c);
@@ -4010,7 +4002,9 @@
ent->checked |= 1;
xmlFree(rep);
rep = NULL;
- }
+ } else {
+ ent->content[0] = 0;
+ }
}
/*