Commit 0fcab658 authored by Nick Wellnhofer's avatar Nick Wellnhofer
Browse files

Handle illegal entity values in recovery mode

Make xmlParseEntityValue always return NULL on error. Otherwise some
illegal entity values like

    <!ENTITY e '&%#4294967298;'>

would later cause problems like integer overflow.

Found by OSS-Fuzz. Should fix bug 783052.

Also see

    https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=592
    https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=2732
parent ca8635bf
......@@ -3741,10 +3741,8 @@ xmlParseEntityValue(xmlParserCtxtPtr ctxt, xmlChar **orig) {
ctxt->instate = XML_PARSER_ENTITY_VALUE;
input = ctxt->input;
GROW;
if (ctxt->instate == XML_PARSER_EOF) {
xmlFree(buf);
return(NULL);
}
if (ctxt->instate == XML_PARSER_EOF)
goto error;
NEXT;
c = CUR_CHAR(l);
/*
......@@ -3765,8 +3763,7 @@ xmlParseEntityValue(xmlParserCtxtPtr ctxt, xmlChar **orig) {
tmp = (xmlChar *) xmlRealloc(buf, size * sizeof(xmlChar));
if (tmp == NULL) {
xmlErrMemory(ctxt, NULL);
xmlFree(buf);
return(NULL);
goto error;
}
buf = tmp;
}
......@@ -3781,10 +3778,13 @@ xmlParseEntityValue(xmlParserCtxtPtr ctxt, xmlChar **orig) {
}
}
buf[len] = 0;
if (ctxt->instate == XML_PARSER_EOF) {
xmlFree(buf);
return(NULL);
if (ctxt->instate == XML_PARSER_EOF)
goto error;
if (c != stop) {
xmlFatalErr(ctxt, XML_ERR_ENTITY_NOT_FINISHED, NULL);
goto error;
}
NEXT;
/*
* Raise problem w.r.t. '&' and '%' being used in non-entities
......@@ -3796,20 +3796,25 @@ xmlParseEntityValue(xmlParserCtxtPtr ctxt, xmlChar **orig) {
if ((*cur == '%') || ((*cur == '&') && (cur[1] != '#'))) {
xmlChar *name;
xmlChar tmp = *cur;
int nameOk = 0;
cur++;
name = xmlParseStringName(ctxt, &cur);
if ((name == NULL) || (*cur != ';')) {
if (name != NULL) {
nameOk = 1;
xmlFree(name);
}
if ((nameOk == 0) || (*cur != ';')) {
xmlFatalErrMsgInt(ctxt, XML_ERR_ENTITY_CHAR_ERROR,
"EntityValue: '%c' forbidden except for entities references\n",
tmp);
goto error;
}
if ((tmp == '%') && (ctxt->inSubset == 1) &&
(ctxt->inputNr == 1)) {
xmlFatalErr(ctxt, XML_ERR_ENTITY_PE_INTERNAL, NULL);
goto error;
}
if (name != NULL)
xmlFree(name);
if (*cur == 0)
break;
}
......@@ -3818,28 +3823,24 @@ xmlParseEntityValue(xmlParserCtxtPtr ctxt, xmlChar **orig) {
/*
* Then PEReference entities are substituted.
*
* NOTE: 4.4.7 Bypassed
* When a general entity reference appears in the EntityValue in
* an entity declaration, it is bypassed and left as is.
* so XML_SUBSTITUTE_REF is not set here.
*/
if (c != stop) {
xmlFatalErr(ctxt, XML_ERR_ENTITY_NOT_FINISHED, NULL);
xmlFree(buf);
} else {
NEXT;
/*
* NOTE: 4.4.7 Bypassed
* When a general entity reference appears in the EntityValue in
* an entity declaration, it is bypassed and left as is.
* so XML_SUBSTITUTE_REF is not set here.
*/
++ctxt->depth;
ret = xmlStringDecodeEntities(ctxt, buf, XML_SUBSTITUTE_PEREF,
0, 0, 0);
--ctxt->depth;
if (orig != NULL)
*orig = buf;
else
xmlFree(buf);
++ctxt->depth;
ret = xmlStringDecodeEntities(ctxt, buf, XML_SUBSTITUTE_PEREF,
0, 0, 0);
--ctxt->depth;
if (orig != NULL) {
*orig = buf;
buf = NULL;
}
error:
if (buf != NULL)
xmlFree(buf);
return(ret);
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment