Re: graphicsmagick packaging
I have a packaged version of graphicsmagick ready for testing:
https://people.debian.org/~bam/debian/pool/main/g/graphicsmagick/
I have attached the debdiff patch to this email. Note that before doing
this upload, I renamed the last patch to CVE-2016-7800.patch, which was
incorrect in the above upload.
It has fixes for the following security issues:
* CVE-2016-7446: heap buffer overflow issue in MVG/SVG rendering.
* CVE-2016-7447: heap overflow of the EscapeParenthesis() function
* CVE-2016-7449: TIFF related problems due to use of strlcpy use.
* CVE-2016-7800: Fix unsigned underflow leading to heap overflow when
parsing 8BIM chunk.
It also has patches that rewrites EscapeString() and MagickGetToken().
To me it looks like the author was not able to reproduce some of these
security issues, and rewrote these functions "just in case" (although
these functions weren't specifically mentioned). From the upstream
annoucement at http://www.graphicsmagick.org/NEWS.html#september-5-2016:
"EscapeParenthesis(): I was notified by Gustavo Grieco of a heap
overflow in EscapeParenthesis() used in the text annotation
code. While not being able to reproduce the issue, the
implementation of this function is completely redone. This issue was
assigned CVE-2016-7447 after the release."
I have not fixed CVE-2016-7448 - the patched code relies on rle_header,
which is not defined for this function in the wheezy version. Might be
possible to fix this, not sure.
There was also another set of changes in the patch for CVE-2016-7446 at
http://hg.graphicsmagick.org/hg/GraphicsMagick/rev/6071b5820215 that I
did not apply, as they depend on adding a new message ("primitive
arithmetic overflow") which involves renumbering all the messages, and
the patch to render.c is somewhat complicated and didn't apply cleanly
either.
In any case I believe CVE-2016-7446 was for the "string may not be null
terminated" error and not the "arithmetic overflow" error.
Regards
--
Brian May <bam@debian.org>
diff -Nru graphicsmagick-1.3.16/debian/changelog graphicsmagick-1.3.16/debian/changelog
--- graphicsmagick-1.3.16/debian/changelog 2016-09-21 07:52:26.000000000 +1000
+++ graphicsmagick-1.3.16/debian/changelog 2016-10-04 17:57:03.000000000 +1100
@@ -1,3 +1,14 @@
+graphicsmagick (1.3.16-1.1+deb7u4) wheezy-security; urgency=high
+
+ * Non-maintainer upload by the LTS Team.
+ * CVE-2016-7446: heap buffer overflow issue in MVG/SVG rendering.
+ * CVE-2016-7447: heap overflow of the EscapeParenthesis() function
+ * CVE-2016-7449: TIFF related problems due to use of strlcpy use.
+ * CVE-2016-7800: Fix unsigned underflow leading to heap overflow when
+ parsing 8BIM chunk.
+
+ -- Brian May <bam@debian.org> Mon, 03 Oct 2016 18:02:36 +1100
+
graphicsmagick (1.3.16-1.1+deb7u3) wheezy-security; urgency=high
* CVE-2016-5240: Prevent denial-of-service by detecting and rejecting
diff -Nru graphicsmagick-1.3.16/debian/patches/CVE-2016-7446.patch graphicsmagick-1.3.16/debian/patches/CVE-2016-7446.patch
--- graphicsmagick-1.3.16/debian/patches/CVE-2016-7446.patch 1970-01-01 10:00:00.000000000 +1000
+++ graphicsmagick-1.3.16/debian/patches/CVE-2016-7446.patch 2016-10-03 17:37:38.000000000 +1100
@@ -0,0 +1,13 @@
+--- a/coders/svg.c
++++ b/coders/svg.c
+@@ -2718,8 +2718,9 @@
+ SAXHandler=(&SAXModules);
+ svg_info.parser=xmlCreatePushParserCtxt(SAXHandler,&svg_info,(char *) NULL,0,
+ image->filename);
+- while ((n=ReadBlob(image,MaxTextExtent,message)) != 0)
++ while ((n=ReadBlob(image,MaxTextExtent-1,message)) != 0)
+ {
++ message[n]='\0';
+ status=xmlParseChunk(svg_info.parser,message,(int) n,False);
+ if (status != 0)
+ break;
diff -Nru graphicsmagick-1.3.16/debian/patches/CVE-2016-7447.patch graphicsmagick-1.3.16/debian/patches/CVE-2016-7447.patch
--- graphicsmagick-1.3.16/debian/patches/CVE-2016-7447.patch 1970-01-01 10:00:00.000000000 +1000
+++ graphicsmagick-1.3.16/debian/patches/CVE-2016-7447.patch 2016-10-04 17:59:20.000000000 +1100
@@ -0,0 +1,69 @@
+--- a/magick/annotate.c
++++ b/magick/annotate.c
+@@ -1412,34 +1412,46 @@
+ %
+ */
+
+-static char *EscapeParenthesis(const char *text)
++static char *EscapeParenthesis(const char *source)
+ {
+ char
+- *buffer;
++ *destination;
+
+ register char
++ *q;
++
++ register const char
+ *p;
+
+- register long
+- i;
++ size_t
++ length;
+
+- unsigned long
+- escapes;
++ assert(source != (const char *) NULL);
+
+- escapes=0;
+- buffer=AllocateString(text);
+- p=buffer;
+- for (i=0; i < (long) Min(strlen(text),(MaxTextExtent-escapes-1)); i++)
+- {
+- if ((text[i] == '(') || (text[i] == ')'))
+- {
+- *p++='\\';
+- escapes++;
+- }
+- *p++=text[i];
+- }
+- *p='\0';
+- return(buffer);
++ /*
++ Use dry-run method to compute required string length.
++ */
++ length=0;
++ for (p=source; *p; p++)
++ {
++ if ((*p == '(') || (*p == ')'))
++ length++;
++ length++;
++ }
++ destination=MagickAllocateMemory(char *,length+1);
++ if (destination == (char *) NULL)
++ MagickFatalError3(ResourceLimitFatalError,MemoryAllocationFailed,
++ UnableToEscapeString);
++ *destination='\0';
++ q=destination;
++ for (p=source; *p; p++)
++ {
++ if ((*p == '(') || (*p == ')'))
++ *q++= '\\';
++ *q++=(*p);
++ }
++ *q=0;
++ return(destination);
+ }
+
+ static MagickPassFail RenderPostscript(Image *image,const DrawInfo *draw_info,
diff -Nru graphicsmagick-1.3.16/debian/patches/CVE-2016-7449.patch graphicsmagick-1.3.16/debian/patches/CVE-2016-7449.patch
--- graphicsmagick-1.3.16/debian/patches/CVE-2016-7449.patch 1970-01-01 10:00:00.000000000 +1000
+++ graphicsmagick-1.3.16/debian/patches/CVE-2016-7449.patch 2016-10-03 18:05:15.000000000 +1100
@@ -0,0 +1,76 @@
+--- a/coders/tiff.c
++++ b/coders/tiff.c
+@@ -1283,6 +1283,19 @@
+ }
+ #endif
+
++/*
++ Copy a possibly unterminated sized string to an image attribute.
++*/
++#define CopySizedFieldToAttribute(key,count,text) \
++ do \
++ { \
++ char _attribute[MaxTextExtent]; \
++ (void) memcpy(_attribute,text,Min(sizeof(_attribute),count)); \
++ _attribute[Min(sizeof(_attribute)-1,count)]='\0'; \
++ (void) SetImageAttribute(image,key,_attribute); \
++ } while(0);
++
++
+ typedef enum
+ {
+ ScanLineMethod, /* Scanline method */
+@@ -1342,6 +1355,7 @@
+ units;
+
+ uint32
++ count,
+ height,
+ rows_per_strip,
+ width;
+@@ -1715,34 +1729,17 @@
+ if (TIFFGetField(tiff,TIFFTAG_SOFTWARE,&text) == 1)
+ (void) SetImageAttribute(image,"software",text);
+
+- {
+- /*
+- "Unsupported" tags return two arguments.
+- */
+-
+- uint32
+- count;
+-
+- char attribute[MaxTextExtent];
+-
+- if (TIFFGetField(tiff,TIFFTAG_OPIIMAGEID,&count,&text) == 1)
+- {
+- (void) strlcpy(attribute,text,Min(sizeof(attribute),(count+1)));
+- (void) SetImageAttribute(image,"imageid",attribute);
+- }
+-
+- if (TIFFGetField(tiff,33423,&count,&text) == 1)
+- {
+- (void) strlcpy(attribute,text,Min(sizeof(attribute),(count+1)));
+- (void) SetImageAttribute(image,"kodak-33423",attribute);
+- }
+-
+- if (TIFFGetField(tiff,36867,&count,&text) == 1)
+- {
+- (void) strlcpy(attribute,text,Min(sizeof(attribute),(count+1)));
+- (void) SetImageAttribute(image,"kodak-36867",attribute);
+- }
+- }
++ /*
++ "Unsupported" tags return two arguments.
++ */
++ if (TIFFGetField(tiff,TIFFTAG_OPIIMAGEID,&count,&text) == 1)
++ CopySizedFieldToAttribute("imageid",count,text);
++
++ if (TIFFGetField(tiff,33423,&count,&text) == 1)
++ CopySizedFieldToAttribute("kodak-33423",count,text);
++
++ if (TIFFGetField(tiff,36867,&count,&text) == 1)
++ CopySizedFieldToAttribute("kodak-36867",count,text);
+
+ if ((photometric == PHOTOMETRIC_PALETTE) ||
+ ((photometric == PHOTOMETRIC_MINISWHITE ||
diff -Nru graphicsmagick-1.3.16/debian/patches/CVE-2016-7800.patch graphicsmagick-1.3.16/debian/patches/CVE-2016-7800.patch
--- graphicsmagick-1.3.16/debian/patches/CVE-2016-7800.patch 1970-01-01 10:00:00.000000000 +1000
+++ graphicsmagick-1.3.16/debian/patches/CVE-2016-7800.patch 2016-10-04 17:44:38.000000000 +1100
@@ -0,0 +1,40 @@
+--- a/coders/meta.c
++++ b/coders/meta.c
+@@ -381,10 +381,17 @@
+ {
+ if (brkused && next > 0)
+ {
++ size_t
++ codes_len;
++
+ char
+ *s = &token[next-1];
+
+- len -= convertHTMLcodes(s, strlen(s));
++ codes_len = convertHTMLcodes(s, strlen(s));
++ if (codes_len > len)
++ len = 0;
++ else
++ len -= codes_len;
+ }
+ }
+
+@@ -646,10 +653,17 @@
+ {
+ if (brkused && next > 0)
+ {
++ size_t
++ codes_len;
++
+ char
+ *s = &token[next-1];
+
+- len -= convertHTMLcodes(s, strlen(s));
++ codes_len = convertHTMLcodes(s, strlen(s));
++ if (codes_len > len)
++ len = 0;
++ else
++ len -= codes_len;
+ }
+ }
+
diff -Nru graphicsmagick-1.3.16/debian/patches/rewrite_EscapeString.patch graphicsmagick-1.3.16/debian/patches/rewrite_EscapeString.patch
--- graphicsmagick-1.3.16/debian/patches/rewrite_EscapeString.patch 1970-01-01 10:00:00.000000000 +1000
+++ graphicsmagick-1.3.16/debian/patches/rewrite_EscapeString.patch 2016-10-04 17:59:37.000000000 +1100
@@ -0,0 +1,23 @@
+--- a/magick/utility.c
++++ b/magick/utility.c
+@@ -758,11 +758,17 @@
+ length;
+
+ assert(source != (const char *) NULL);
+- length=strlen(source)+1;
++ /*
++ Use dry-run method to compute required string length.
++ */
++ length=0;
+ for (p=source; *p; p++)
+- if ((*p == '\\') || (*p == escape))
++ {
++ if ((*p == '\\') || (*p == escape))
++ length++;
+ length++;
+- destination=MagickAllocateMemory(char *,length);
++ }
++ destination=MagickAllocateMemory(char *,length+1);
+ if (destination == (char *) NULL)
+ MagickFatalError3(ResourceLimitFatalError,MemoryAllocationFailed,
+ UnableToEscapeString);
diff -Nru graphicsmagick-1.3.16/debian/patches/rewrite_MagickGetToken.patch graphicsmagick-1.3.16/debian/patches/rewrite_MagickGetToken.patch
--- graphicsmagick-1.3.16/debian/patches/rewrite_MagickGetToken.patch 1970-01-01 10:00:00.000000000 +1000
+++ graphicsmagick-1.3.16/debian/patches/rewrite_MagickGetToken.patch 2016-10-03 17:39:14.000000000 +1100
@@ -0,0 +1,175 @@
+--- a/magick/utility.c
++++ b/magick/utility.c
+@@ -3453,85 +3453,91 @@
+ p=(char *) start;
+
+ if (*p != '\0')
+- {
+- while (isspace((int)(unsigned char) (*p)) && (*p != '\0'))
+- p++;
+- switch (*p)
+ {
+- case '"':
+- case '\'':
+- case '{':
+- {
+- register char
+- escape;
+-
+- escape=(*p);
+- if (escape == '{')
+- escape='}';
+- for (p++; *p != '\0'; p++)
++ while (isspace((int)(unsigned char) (*p)) && (*p != '\0'))
++ p++;
++ switch (*p)
+ {
+- if ((*p == '\\') && ((*(p+1) == escape) || (*(p+1) == '\\')))
+- p++;
+- else
+- if (*p == escape)
+- {
+- p++;
+- break;
+- }
+- if (i < length)
+- token[i++]=(*p);
+- }
+- break;
+- }
+- default:
+- {
+- char
+- *q;
+-
+- double_val=strtod(p,&q);
+- (void) double_val;
+- if (p != q)
++ case '"':
++ case '\'':
++ case '{':
+ {
+- for ( ; p < q; p++)
+- if (i < length)
+- token[i++]=(*p);
+- if (*p == '%')
+- if (i < length)
+- {
+- token[i++]=(*p);
++ register char
++ escape;
++
++ escape=(*p);
++ if (escape == '{')
++ escape='}';
++ for (p++; *p != '\0'; p++)
++ {
++ if ((*p == '\\') && ((*(p+1) == escape) || (*(p+1) == '\\')))
+ p++;
+- }
++ else
++ if (*p == escape)
++ {
++ p++;
++ break;
++ }
++ if (i < length)
++ token[i++]=(*p);
++ }
+ break;
+ }
+- if ((*p != '\0') && !isalpha((int) *p) && (*p != *DirectorySeparator) &&
+- (*p != '#') && (*p != '<'))
++ default:
+ {
+- if (i < length)
++ char
++ *q;
++
++ double_val=strtod(p,&q);
++ (void) double_val;
++ if (p != q)
+ {
+- token[i++]=(*p);
+- p++;
++ for ( ; p < q; p++)
++ if (i < length)
++ token[i++]=(*p);
++ if (*p == '%')
++ if (i < length)
++ {
++ token[i++]=(*p);
++ p++;
++ }
++ break;
++ }
++ if ((*p != '\0') && !isalpha((int) *p) &&
++ (*p != *DirectorySeparator) &&
++ (*p != '#') && (*p != '<'))
++ {
++ if (i < length)
++ {
++ token[i++]=(*p);
++ p++;
++ }
++ break;
++ }
++ for ( ; *p != '\0'; p++)
++ {
++ if ((isspace((int)(unsigned char) *p) ||
++ (*p == '=')) && (*(p-1) != '\\'))
++ break;
++ if (i < length)
++ token[i++]=(*p);
++ if (*p == '(')
++ {
++ for (p++; *p != '\0'; p++)
++ {
++ if (i < length)
++ token[i++]=(*p);
++ if ((*p == ')') && (*(p-1) != '\\'))
++ break;
++ }
++ }
++ if (*p == '\0')
++ break;
+ }
+ break;
+ }
+- for ( ; *p != '\0'; p++)
+- {
+- if ((isspace((int)(unsigned char) *p) || (*p == '=')) && (*(p-1) != '\\'))
+- break;
+- if (i < length)
+- token[i++]=(*p);
+- if (*p == '(')
+- for (p++; *p != '\0'; p++)
+- {
+- if (i < length)
+- token[i++]=(*p);
+- if ((*p == ')') && (*(p-1) != '\\'))
+- break;
+- }
+ }
+- break;
+- }
+ }
+- }
+ token[i]='\0';
+ {
+ char
+@@ -3542,10 +3548,10 @@
+ */
+ if ((LocaleNCompare(token,"url(#",5) == 0) &&
+ ((r = strrchr(token,')')) != NULL))
+- {
+- *r='\0';
+- (void) memmove(token,token+5,r-token+1);
+- }
++ {
++ *r='\0';
++ (void) memmove(token,token+5,r-token+1);
++ }
+ }
+ if (end != (char **) NULL)
+ *end=p;
diff -Nru graphicsmagick-1.3.16/debian/patches/series graphicsmagick-1.3.16/debian/patches/series
--- graphicsmagick-1.3.16/debian/patches/series 2016-09-22 17:52:40.000000000 +1000
+++ graphicsmagick-1.3.16/debian/patches/series 2016-10-05 08:04:51.000000000 +1100
@@ -1,3 +1,9 @@
upstreamdiff.patch
CVE-2016-5240.patch
CVE-2016-5241.patch
+CVE-2016-7446.patch
+rewrite_MagickGetToken.patch
+CVE-2016-7447.patch
+rewrite_EscapeString.patch
+CVE-2016-7449.patch
+CVE-2016-7800.patch
Reply to: