Comment 16 for bug 27952

Message-ID: <email address hidden>
Date: Tue, 10 Jan 2006 19:03:55 +0100
From: Daniel Kobras <email address hidden>
To: <email address hidden>
Cc: Matthias Clasen <email address hidden>
Subject: Re: Bug#345876: imagemagick: New format string vulnerability in SetImageInfo().

On Tue, Jan 10, 2006 at 05:03:55PM +0100, Daniel Kobras wrote:
> On Mon, Jan 09, 2006 at 10:18:13AM -0500, Matthias Clasen wrote:
> > Ah, got it now. Does this look more complete ?
>
> Yes, but there are still a few more places to fix. The attached patch
> for 6.2.4.5 should be fairly complete, apart from a few odd places in
> coders/, but those fall more into the scope of CVE-2005-4601. Mind,
> though, that I'm still testing the fix, so some polishing might be
> needed still.
(...)
> +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> +% %
> +% %
> +% %
> +% F o r m a t M a g i c k S t r i n g N u m e r i c %
> +% %
> +% %
> +% %
> +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> +%
> +% Method FormatMagickStringNumeric formats output for a single numeric
> +% argument. It takes into account that the format string given might be
> +% untrusted user input, and returns the length of the formatted string.
> +%
> +% The format of the FormatMagickStringNumeric method is:
> +%
> +% long FormatMagickStringNumeric(char *string,const size_t length,
> +% const char *format,int value)
> +%
> +% A description of each parameter follows.
> +%
> +% o string: FormatMagickStringNumeric() returns the formatted string in this
> +% character buffer.
> +%
> +% o length: The maximum length of the string.
> +%
> +% 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 long FormatMagickStringNumeric(char *string,const size_t length,const char *format,int value)
> +{
> + char
> + *p;
> +
> + (void) CopyMagickString(string, format, length);
> +
> + 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),length-(p-format),p,value);
> + *q=c;
> + (void) ConcatenateMagickString(&string,q,length);

Bah, scratch that &, even. Forgot to fix that one before sending.

Sorry,

Daniel.