[libxml2] Handle illegal entity values in recovery mode



commit 0fcab658a27c3f0759b89809da1015f9bcbd999a
Author: Nick Wellnhofer <wellnhofer aevum de>
Date:   Thu Sep 7 18:25:11 2017 +0200

    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

 parser.c |   63 +++++++++++++++++++++++++++++++------------------------------
 1 files changed, 32 insertions(+), 31 deletions(-)
---
diff --git a/parser.c b/parser.c
index 1005395..6c40b2e 100644
--- a/parser.c
+++ b/parser.c
@@ -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);
 }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]