Re: Sufficient patch for CVE-2006-0082
Daniel Kobras <[email protected]> Wed, 1 Mar 2006 17:31:09 +0100
| Newsgroups | gmane.comp.video.graphicsmagick.core |
|---|---|
| Message-ID | <[email protected]> |
On Wed, Mar 01, 2006 at 09:57:59AM -0600, Albert Chin wrote: > Is the attached patch ok for CVE-2006-0082? It is taken from Gentoo: > http://www.gentoo.org/security/en/glsa/glsa-200602-13.xml This patch breaks multi-frame support, and misses a number of similar code paths. The attached patch is applied to the Debian package of 1.1.7. I can check in a version for CVS HEAD if someone tells me how to provide proper changelog entries. Are they autogenerated from the commit message? Is the HTML version generated from plaintext or vice versa? Regards, Daniel.
format_string_fix_CAN-2005-0397
(text/plain, 8.5 KB)
Format string security fix, addressing CVE-2005-0397 and further related
problems. Introduces new function FormatStringNumeric() that allows a
single numeric format substitution on untrusted user input.
--- a/magick/blob.c Thu Jan 5 17:11:06 2006 +0100
+++ b/magick/blob.c Thu Jan 5 17:12:40 2006 +0100
@@ -2130,31 +2130,13 @@
static void FormMultiPartFilename(Image *image, const ImageInfo *image_info)
{
char
- filename[MaxTextExtent],
+ *filename,
*p;
/*
Form filename for multi-part images.
*/
- (void) strncpy(filename,image->filename,MaxTextExtent-1);
- for (p=strchr(filename,'%'); p != (char *) NULL; p=strchr(p+1,'%'))
- {
- char
- *q;
-
- q=p+1;
- if (*q == '0')
- (void) strtol(q,&q,10);
- if (*q == 'd')
- {
- char
- format[MaxTextExtent];
-
- (void) strncpy(format,p,MaxTextExtent-1);
- FormatString(p,format,GetImageIndexInList(image));
- break;
- }
- }
+ filename=FormatStringNumeric(image->filename,GetImageIndexInList(image));
if (!image_info->adjoin)
if ((image->previous != (Image *) NULL) ||
(image->next != (Image *) NULL))
@@ -2167,6 +2149,7 @@
MaxTextExtent-1);
}
(void) strncpy(image->filename,filename,MaxTextExtent-1);
+ MagickFreeMemory(filename);
}
MagickExport unsigned int OpenBlob(const ImageInfo *image_info,Image *image,
--- a/magick/command.c Thu Jan 5 17:11:06 2006 +0100
+++ b/magick/command.c Thu Jan 5 17:12:40 2006 +0100
@@ -437,16 +437,17 @@
if (first_scene != last_scene)
{
char
- filename[MaxTextExtent];
+ *filename;
/*
Form filename for multi-part images.
*/
- FormatString(filename,image_info->filename,scene);
+ filename=FormatStringNumeric(image_info->filename,scene);
if (LocaleCompare(filename,image_info->filename) == 0)
FormatString(filename,"%.1024s[%lu]",image_info->filename,
scene);
(void) strncpy(image_info->filename,filename,MaxTextExtent-1);
+ MagickFreeMemory(filename);
}
image_info->colorspace=quantize_info->colorspace;
image_info->dither=quantize_info->dither;
@@ -5337,15 +5338,16 @@
if (first_scene != last_scene)
{
char
- filename[MaxTextExtent];
+ *filename;
/*
Form filename for multi-part images.
*/
- FormatString(filename,image_info->filename,scene);
+ filename=FormatStringNumeric(image_info->filename,scene);
if (LocaleCompare(filename,image_info->filename) == 0)
FormatString(filename,"%.1024s.%lu",image_info->filename,scene);
(void) strncpy(image_info->filename,filename,MaxTextExtent-1);
+ MagickFreeMemory(filename);
}
(void) strcpy(image_info->magick,"MIFF");
image_info->colorspace=quantize_info->colorspace;
@@ -11916,15 +11918,16 @@
if (first_scene != last_scene)
{
char
- filename[MaxTextExtent];
+ *filename;
/*
Form filename for multi-part images.
*/
- FormatString(filename,image_info->filename,scene);
+ filename=FormatStringNumeric(image_info->filename,scene);
if (LocaleCompare(filename,image_info->filename) == 0)
FormatString(filename,"%.1024s.%lu",image_info->filename,scene);
(void) strncpy(image_info->filename,filename,MaxTextExtent-1);
+ MagickFreeMemory(filename);
}
(void) CloneString(&image_info->font,montage_info->font);
image_info->colorspace=quantize_info.colorspace;
--- a/magick/image.c Thu Jan 5 17:11:06 2006 +0100
+++ b/magick/image.c Thu Jan 5 17:12:40 2006 +0100
@@ -6244,16 +6244,19 @@
}
if (rectify)
{
+ char
+ *filename_fmt;
const MagickInfo
*magick_info;
/*
Rectify multi-image file support.
*/
- FormatString(filename,image_info->filename,0);
- if ((LocaleCompare(filename,image_info->filename) != 0) &&
- (strchr(filename,'%') == (char *) NULL))
+ filename_fmt=FormatStringNumeric(image_info->filename,0);
+ if ((LocaleCompare(filename_fmt,image_info->filename) != 0) &&
+ (strchr(filename_fmt,'%') == (char *) NULL))
image_info->adjoin=False;
+ MagickFreeMemory(filename_fmt);
magick_info=GetMagickInfo(magic,exception);
if (magick_info != (const MagickInfo *) NULL)
image_info->adjoin&=magick_info->adjoin;
--- a/magick/log.c Thu Jan 5 17:11:06 2006 +0100
+++ b/magick/log.c Thu Jan 5 17:12:40 2006 +0100
@@ -517,11 +517,11 @@
if (log_info->file == (FILE *) NULL)
{
char
- filename[MaxTextExtent];
-
- FormatString(filename,log_info->filename,
- log_info->generation);
+ *filename;
+
+ filename=FormatStringNumeric(log_info->filename,log_info->generation);
log_info->file=fopen(filename,"w");
+ MagickFreeMemory(filename);
if (log_info->file == (FILE *) NULL)
{
LiberateSemaphoreInfo(&log_semaphore);
@@ -569,11 +569,11 @@
if (log_info->file == (FILE *) NULL)
{
char
- filename[MaxTextExtent];
-
- FormatString(filename,log_info->filename,
- log_info->generation);
+ *filename;
+
+ filename=FormatStringNumeric(log_info->filename,log_info->generation);
log_info->file=fopen(filename,"w");
+ MagickFreeMemory(filename);
if (log_info->file == (FILE *) NULL)
{
LiberateSemaphoreInfo(&log_semaphore);
--- a/magick/utility.c Thu Jan 5 17:11:06 2006 +0100
+++ b/magick/utility.c Thu Jan 5 17:12:40 2006 +0100
@@ -1127,6 +1127,72 @@
# endif
#endif
va_end(operands);
+}
+
+/*
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+% %
+% %
+% %
+% F o r m a t S t r i n g N u m e r i c %
+% %
+% %
+% %
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+%
+% Method FormatStringNumeric formats output for a single numeric argument.
+% It takes into account that the format string given might be untrusted
+% user input, and returns a pointer to the formatted string.
+%
+% The format of the FormatStringNumeric method is:
+%
+% char * FormatStringNumeric(char *string,const char *format,int value)
+%
+% A description of each parameter follows.
+%
+% o format: A string describing the format to use to write the numeric
+% argument. Only the first numeric format identifier is replaced.
+%
+% o value: Numeric value to substitute into format string.
+%
+%
+*/
+MagickExport char *FormatStringNumeric(const char *format,int value)
+{
+ char
+ *p,
+ *string;
+
+ string = NULL;
+
+ (void) CloneString(&string, format);
+
+ for (p=strchr(format,'%'); p != (char *) NULL; p=strchr(p+1,'%'))
+ {
+ char
+ *q;
+
+ q=(char *) p+1;
+ if (*q == '0')
+ (void) strtol(q,&q,10);
+ if ((*q == '%') || (*q == 'd') || (*q == 'o') || (*q == 'x'))
+ {
+ char
+ c;
+
+ q++;
+ c=*q;
+ *q='\0';
+ (void) snprintf(string+(p-format),MaxTextExtent-(p-format),p,value);
+ *q=c;
+ (void) ConcatenateString(&string,q);
+ if (*(q-1) == '%')
+ p++;
+ else
+ break;
+ }
+ }
+ return string;
}
/*
--- a/magick/utility.h Thu Jan 5 17:11:06 2006 +0100
+++ b/magick/utility.h Thu Jan 5 17:12:40 2006 +0100
@@ -61,6 +61,7 @@
*AllocateString(const char *),
*Base64Encode(const unsigned char *,const size_t,size_t *),
*EscapeString(const char *,const char),
+ *FormatStringNumeric(const char *,int),
*GetPageGeometry(const char *),
**ListFiles(const char *,const char *,long *),
*SetClientName(const char *),