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.
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: %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%% %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%%%%%% %%% ingNumeric formats output for a single numeric ingNumeric method is: ingNumeric( char *string,const size_t length, ingNumeric( ) returns the formatted string in this ingNumeric( char *string,const size_t length,const char *format,int value) g(string, format, length); format, '%'); p != (char *) NULL; p=strchr(p+1,'%')) string+ (p-format) ,length- (p-format) ,p,value) ; ckString( &string, q,length) ;
> 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 FormatMagickStr
> +% 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 FormatMagickStr
> +%
> +% long FormatMagickStr
> +% const char *format,int value)
> +%
> +% A description of each parameter follows.
> +%
> +% o string: FormatMagickStr
> +% 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 FormatMagickStr
> +{
> + char
> + *p;
> +
> + (void) CopyMagickStrin
> +
> + for (p=strchr(
> + {
> + 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(
> + *q=c;
> + (void) ConcatenateMagi
Bah, scratch that &, even. Forgot to fix that one before sending.
Sorry,
Daniel.