SVG: Fix buffer-overflow and inconsistent behavior in GetStyleTokens().
authorBob Friesenhahn <bfriesen@GraphicsMagick.org>
Wed, 16 Aug 2017 08:28:19 -0500
changeset 15121 54f48ab2d52a
parent 15120 4970ea920a93
child 15122 b6c54b2d5991
SVG: Fix buffer-overflow and inconsistent behavior in GetStyleTokens().
ChangeLog
VisualMagick/installer/inc/version.isx
coders/svg.c
magick/version.h
www/Changelog.html
--- a/ChangeLog	Tue Aug 15 08:05:00 2017 -0500
+++ b/ChangeLog	Wed Aug 16 08:28:19 2017 -0500
@@ -1,3 +1,15 @@
+2017-08-16  Bob Friesenhahn  <bfriesen@simple.dallas.tx.us>
+
+	* coders/svg.c (GetStyleTokens): Fix SourceForge bugs 434 "heap
+	buffer overflow in GetStyleTokens", 435 "null pointer
+	dereference_in_SVGStartElement", and 436 "heap buffer overflow in
+	GetStyleTokens" which all originated from a heap buffer overflow
+	in GetStyleStokens(), or inconsistent initialization.  Now the
+	implementation truncates parsing for poorly-formed input (to avoid
+	buffer overflow) while still correctly parsing well-formed input.
+	The reproducers and problem reports are attributed to "ADLab of
+	Venustech".
+
 2017-08-14  Glenn Randers-Pehrson  <glennrp@simple.dallas.tx.us>
 
 	* coders/png.c (ReadOneJNGImage): Fixed double-free after
--- a/VisualMagick/installer/inc/version.isx	Tue Aug 15 08:05:00 2017 -0500
+++ b/VisualMagick/installer/inc/version.isx	Wed Aug 16 08:28:19 2017 -0500
@@ -10,5 +10,5 @@
 
 #define public MagickPackageName "GraphicsMagick"
 #define public MagickPackageVersion "1.4"
-#define public MagickPackageVersionAddendum ".020170814"
-#define public MagickPackageReleaseDate "snapshot-20170814"
+#define public MagickPackageVersionAddendum ".020170816"
+#define public MagickPackageReleaseDate "snapshot-20170816"
--- a/coders/svg.c	Tue Aug 15 08:05:00 2017 -0500
+++ b/coders/svg.c	Wed Aug 16 08:28:19 2017 -0500
@@ -267,11 +267,12 @@
   char
     **tokens;
 
-  register const char
+  const char
     *p,
     *q;
 
-  register size_t
+  size_t
+    alloc_tokens,
     i;
 
   SVGInfo
@@ -279,21 +280,27 @@
 
   svg_info=(SVGInfo *) context;
   *number_tokens=0;
+  alloc_tokens=0;
   if (text == (const char *) NULL)
     return((char **) NULL);
   /*
     Determine the number of arguments.
+
+    style="fill: red; stroke: blue; stroke-width: 3"
   */
   for (p=text; *p != '\0'; p++)
     if (*p == ':')
-      (*number_tokens)+=2;
-  tokens=MagickAllocateMemory(char **,(*number_tokens+2)*sizeof(*tokens));
+      alloc_tokens+=2;
+  if (alloc_tokens == 0)
+    return((char **) NULL);
+  tokens=MagickAllocateMemory(char **,(alloc_tokens+2)*sizeof(*tokens));
   if (tokens == (char **) NULL)
     {
       ThrowException3(svg_info->exception,ResourceLimitError,
                       MemoryAllocationFailed,UnableToConvertStringToTokens);
       return((char **) NULL);
     }
+  (void) memset(tokens,0,(alloc_tokens+2)*sizeof(*tokens));
   /*
     Convert string to an ASCII list.
   */
@@ -304,14 +311,36 @@
       if ((*q != ':') && (*q != ';') && (*q != '\0'))
         continue;
       tokens[i]=AllocateString(p);
+      if (tokens[i] == NULL)
+        {
+          ThrowException3(svg_info->exception,ResourceLimitError,
+                          MemoryAllocationFailed,UnableToConvertStringToTokens);
+          break;
+        }
       (void) strlcpy(tokens[i],p,q-p+1);
-      Strip(tokens[i++]);
+      Strip(tokens[i]);
+      i++;
+      if (i >= alloc_tokens)
+        break;
       p=q+1;
     }
-  tokens[i]=AllocateString(p);
-  (void) strlcpy(tokens[i],p,q-p+1);
-  Strip(tokens[i++]);
+  if (i < alloc_tokens)
+    {
+      tokens[i]=AllocateString(p);
+      if (tokens[i] == NULL)
+        {
+          ThrowException3(svg_info->exception,ResourceLimitError,
+                          MemoryAllocationFailed,UnableToConvertStringToTokens);
+        }
+      else
+        {
+          (void) strlcpy(tokens[i],p,q-p+1);
+          Strip(tokens[i]);
+          i++;
+        }
+    }
   tokens[i]=(char *) NULL;
+  *number_tokens=i;
   return(tokens);
 }
 
--- a/magick/version.h	Tue Aug 15 08:05:00 2017 -0500
+++ b/magick/version.h	Wed Aug 16 08:28:19 2017 -0500
@@ -38,8 +38,8 @@
 #define MagickLibVersion  0x191600
 #define MagickLibVersionText  "1.4"
 #define MagickLibVersionNumber 19,16,0
-#define MagickChangeDate   "20170814"
-#define MagickReleaseDate  "snapshot-20170814"
+#define MagickChangeDate   "20170816"
+#define MagickReleaseDate  "snapshot-20170816"
 	
 /*
   The MagickLibInterfaceNewest and MagickLibInterfaceOldest defines
--- a/www/Changelog.html	Tue Aug 15 08:05:00 2017 -0500
+++ b/www/Changelog.html	Wed Aug 16 08:28:19 2017 -0500
@@ -35,6 +35,20 @@
 <div class="document">
 
 
+<p>2017-08-16  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 (GetStyleTokens): Fix SourceForge bugs 434 &quot;heap
+buffer overflow in GetStyleTokens&quot;, 435 &quot;null pointer
+dereference_in_SVGStartElement&quot;, and 436 &quot;heap buffer overflow in
+GetStyleTokens&quot; which all originated from a heap buffer overflow
+in GetStyleStokens(), or inconsistent initialization.  Now the
+implementation truncates parsing for poorly-formed input (to avoid
+buffer overflow) while still correctly parsing well-formed input.
+The reproducers and problem reports are attributed to &quot;ADLab of
+Venustech&quot;.</li>
+</ul>
+</blockquote>
 <p>2017-08-14  Glenn Randers-Pehrson  &lt;<a class="reference external" href="mailto:glennrp&#37;&#52;&#48;simple&#46;dallas&#46;tx&#46;us">glennrp<span>&#64;</span>simple<span>&#46;</span>dallas<span>&#46;</span>tx<span>&#46;</span>us</a>&gt;</p>
 <blockquote>
 <ul class="simple">