GraphicsMagick: SVG: Check parser context alloc. Disable parse ...

changeset 88699a41798c in /hg/GraphicsMagick
details: http://hg.GraphicsMagick.org/hg/GraphicsMagick?cmd=changeset;node=88699a41798c
summary: SVG: Check parser context alloc.  Disable parse internal subset.

diffstat:

 ChangeLog                              |  10 +++++
 VisualMagick/installer/inc/version.isx |   4 +-
 coders/svg.c                           |  59 +++++++++++++++++++++------------
 magick/version.h                       |   4 +-
 www/Changelog.html                     |  12 ++++++
 5 files changed, 64 insertions(+), 25 deletions(-)

diffs (186 lines):

diff -r 5de8e986e4c4 -r 88699a41798c ChangeLog
--- a/ChangeLog	Thu Aug 11 08:31:14 2022 -0500
+++ b/ChangeLog	Sat Aug 13 09:51:50 2022 -0500
@@ -1,3 +1,13 @@
+2022-08-13  Bob Friesenhahn  <[email protected]>
+
+	* coders/svg.c (ReadSVGImage): Address concern from SourceForge
+	issue #669 "Segmentation fault caused by null pointer dereference
+	by checking return from xmlCreatePushParserCtxt().  Address
+	oss-fuzz 48340 "graphicsmagick:coder_SVG_fuzzer:
+	Heap-use-after-free in xmlParseInternalSubset" by disabling
+	internal subset handling until the parser context handling is
+	fixed.
+
 2022-08-11  Bob Friesenhahn  <[email protected]>
 
 	* NEWS.txt: Updated the news.
diff -r 5de8e986e4c4 -r 88699a41798c VisualMagick/installer/inc/version.isx
--- a/VisualMagick/installer/inc/version.isx	Thu Aug 11 08:31:14 2022 -0500
+++ b/VisualMagick/installer/inc/version.isx	Sat Aug 13 09:51:50 2022 -0500
@@ -10,5 +10,5 @@
 
 #define public MagickPackageName "GraphicsMagick"
 #define public MagickPackageVersion "1.4"
-#define public MagickPackageVersionAddendum ".020220811"
-#define public MagickPackageReleaseDate "snapshot-20220811"
+#define public MagickPackageVersionAddendum ".020220813"
+#define public MagickPackageReleaseDate "snapshot-20220813"
diff -r 5de8e986e4c4 -r 88699a41798c coders/svg.c
--- a/coders/svg.c	Thu Aug 11 08:31:14 2022 -0500
+++ b/coders/svg.c	Sat Aug 13 09:51:50 2022 -0500
@@ -494,13 +494,17 @@
 
   static int SVGIsStandalone(void *context);
 
+#if defined(ENABLE_XML_INTERNAL_SUBSET)
   static int SVGHasInternalSubset(void *context);
+#endif /* ENABLE_XML_INTERNAL_SUBSET */
 
   static int SVGHasExternalSubset(void *context);
 
+#if defined(ENABLE_XML_INTERNAL_SUBSET)
   static void SVGInternalSubset(void *context,const xmlChar *name,
                                 const xmlChar *external_id,
                                 const xmlChar *system_id);
+#endif /* ENABLE_XML_INTERNAL_SUBSET */
 
   static xmlParserInputPtr SVGResolveEntity(void *context,
                                             const xmlChar *public_id,
@@ -587,6 +591,7 @@
   return(svg_info->document->standalone == 1);
 }
 
+#if defined(ENABLE_XML_INTERNAL_SUBSET) && ENABLE_XML_INTERNAL_SUBSET
 static int
 SVGHasInternalSubset(void *context)
 {
@@ -601,6 +606,7 @@
   svg_info=(SVGInfo *) context;
   return(svg_info->document->intSubset != NULL);
 }
+#endif /* ENABLE_XML_INTERNAL_SUBSET */
 
 static int
 SVGHasExternalSubset(void *context)
@@ -617,6 +623,8 @@
   return(svg_info->document->extSubset != NULL);
 }
 
+#if defined(ENABLE_XML_INTERNAL_SUBSET) && ENABLE_XML_INTERNAL_SUBSET
+/* FIXME: Parser context allocation/handling is apparently wrong for internal subset */
 static void
 SVGInternalSubset(void *context,const xmlChar *name,
                   const xmlChar *external_id,const xmlChar *system_id)
@@ -634,6 +642,7 @@
   svg_info=(SVGInfo *) context;
   (void) xmlCreateIntSubset(svg_info->document,name,external_id,system_id);
 }
+#endif /* ENABLE_XML_INTERNAL_SUBSET */
 
 static xmlParserInputPtr
 SVGResolveEntity(void *context,
@@ -4077,9 +4086,13 @@
   (void) xmlSubstituteEntitiesDefault(0);
 
   (void) memset(&SAXModules,0,sizeof(SAXModules));
+#if defined(ENABLE_XML_INTERNAL_SUBSET) && ENABLE_XML_INTERNAL_SUBSET
   SAXModules.internalSubset=SVGInternalSubset;
+#endif /* ENABLE_XML_INTERNAL_SUBSET */
   SAXModules.isStandalone=SVGIsStandalone;
+#if defined(ENABLE_XML_INTERNAL_SUBSET) && ENABLE_XML_INTERNAL_SUBSET
   SAXModules.hasInternalSubset=SVGHasInternalSubset;
+#endif /* ENABLE_XML_INTERNAL_SUBSET */
   SAXModules.hasExternalSubset=SVGHasExternalSubset;
   SAXModules.resolveEntity=SVGResolveEntity;
   SAXModules.getEntity=SVGGetEntity;
@@ -4110,28 +4123,32 @@
                                           image->filename);
   if (svg_info.parser == (xmlParserCtxtPtr) NULL)
     {
-      /* FIXME: Handle failure! */
-    }
-  while ((n=ReadBlob(image,MaxTextExtent-1,message)) != 0)
-    {
-      message[n]='\0';
-      status=xmlParseChunk(svg_info.parser,message,(int) n,False);
-      if (status != 0)
-        break;
+      ThrowException(exception,DrawError,UnableToDrawOnImage,
+                     "Failed to push XML parser context");
     }
-  (void) xmlParseChunk(svg_info.parser,message,0,True);
-  /*
-    Assure that our private context is freed, even if we abort before
-    seeing the document end.
-  */
-  SVGEndDocument(&svg_info);
-  if (svg_info.parser->myDoc != (xmlDocPtr) NULL)
-    xmlFreeDoc(svg_info.parser->myDoc);
-  /*
-    Free all the memory used by a parser context. However the parsed
-    document in ctxt->myDoc is not freed (so we just did that).
-  */
-  xmlFreeParserCtxt(svg_info.parser);
+  if (svg_info.parser != (xmlParserCtxtPtr) NULL)
+    {
+      while ((n=ReadBlob(image,MaxTextExtent-1,message)) != 0)
+        {
+          message[n]='\0';
+          status=xmlParseChunk(svg_info.parser,message,(int) n,False);
+          if (status != 0)
+            break;
+        }
+      (void) xmlParseChunk(svg_info.parser,message,0,True);
+      /*
+        Assure that our private context is freed, even if we abort before
+        seeing the document end.
+      */
+      SVGEndDocument(&svg_info);
+      if (svg_info.parser->myDoc != (xmlDocPtr) NULL)
+        xmlFreeDoc(svg_info.parser->myDoc);
+      /*
+        Free all the memory used by a parser context. However the parsed
+        document in ctxt->myDoc is not freed (so we just did that).
+      */
+      xmlFreeParserCtxt(svg_info.parser);
+    }
   (void) LogMagickEvent(CoderEvent,GetMagickModule(),"end SAX");
   (void) fclose(file);
   CloseBlob(image);
diff -r 5de8e986e4c4 -r 88699a41798c magick/version.h
--- a/magick/version.h	Thu Aug 11 08:31:14 2022 -0500
+++ b/magick/version.h	Sat Aug 13 09:51:50 2022 -0500
@@ -38,8 +38,8 @@
 #define MagickLibVersion  0x272400
 #define MagickLibVersionText  "1.4"
 #define MagickLibVersionNumber 27,24,0
-#define MagickChangeDate   "20220811"
-#define MagickReleaseDate  "snapshot-20220811"
+#define MagickChangeDate   "20220813"
+#define MagickReleaseDate  "snapshot-20220813"
 
 /*
   The MagickLibInterfaceNewest and MagickLibInterfaceOldest defines
diff -r 5de8e986e4c4 -r 88699a41798c www/Changelog.html
--- a/www/Changelog.html	Thu Aug 11 08:31:14 2022 -0500
+++ b/www/Changelog.html	Sat Aug 13 09:51:50 2022 -0500
@@ -35,6 +35,18 @@
 <div class="document">
 
 
+<p>2022-08-13  Bob Friesenhahn  &lt;<a class="reference external" href="mailto:bfriesen&#37;&#52;&#48;simple&#46;dallas&#46;tx&#46;us">bfriesen<span>&#64;</span>simple<span>&#46;</span>dallas<span>&#46;</span>tx<span>&#46;</span>us</a>&gt;</p>
+<blockquote>
+<ul class="simple">
+<li>coders/svg.c (ReadSVGImage): Address concern from SourceForge
+issue #669 &quot;Segmentation fault caused by null pointer dereference
+by checking return from xmlCreatePushParserCtxt().  Address
+oss-fuzz 48340 &quot;graphicsmagick:coder_SVG_fuzzer:
+Heap-use-after-free in xmlParseInternalSubset&quot; by disabling
+internal subset handling until the parser context handling is
+fixed.</li>
+</ul>
+</blockquote>
 <p>2022-08-11  Bob Friesenhahn  &lt;<a class="reference external" href="mailto:bfriesen&#37;&#52;&#48;simple&#46;dallas&#46;tx&#46;us">bfriesen<span>&#64;</span>simple<span>&#46;</span>dallas<span>&#46;</span>tx<span>&#46;</span>us</a>&gt;</p>
 <blockquote>
 <ul class="simple">