Re: Sufficient patch for CVE-2005-4601?

On Wed, 1 Mar 2006, Albert Chin wrote:

> Is the attached patch ok for CVE-2005-4601? It is taken from the
> Debian patch for ImageMagick 5.4.4.5.

I think that it is ok, but it is by no means ideal since it limits the 
available range of file names.  It is worth noting that the proper way 
to update the locale file is to cd to the 'locale' subdirectory, edit 
C.mgk, update the messages, and type 'make install'.  This requires 
that a version of gm is already in the path since gm is used to 
transform into the output files.

Bob

> --
> albert chin ([email protected])
>
> -- snip snip
> Index: magick/delegate.c
> ===================================================================
> RCS file: /GraphicsMagick/GraphicsMagick/magick/delegate.c,v
> retrieving revision 1.124
> diff -u -3 -p -r1.124 delegate.c
> --- magick/delegate.c	10 May 2005 17:07:21 -0000	1.124
> +++ magick/delegate.c	1 Mar 2006 15:53:40 -0000
> @@ -337,6 +337,8 @@ MagickExport const DelegateInfo *GetDele
> %
> %
> */
> +#define ProhibitedAlphabet  "*?\"'<>|`$"
> +
> MagickExport unsigned int InvokeDelegate(ImageInfo *image_info,Image *image,
>   const char *decode,const char *encode,ExceptionInfo *exception)
> {
> @@ -399,9 +401,9 @@ MagickExport unsigned int InvokeDelegate
>       image_info->temporary=True;
>     }
>
> -  if (delegate_info->mode != 0)
> -    if ((decode && (delegate_info->encode != (char *) NULL)) ||
> -        (encode && (delegate_info->decode != (char *) NULL)))
> +  if ((delegate_info->mode != 0) &&
> +       ((decode && (delegate_info->encode != (char *) NULL)) ||
> +        (encode && (delegate_info->decode != (char *) NULL))))
>       {
>         char
>           filename[MaxTextExtent],
> @@ -433,6 +435,13 @@ MagickExport unsigned int InvokeDelegate
>           (void) ThrowException(exception,FileOpenError,UnableToCreateTemporaryFile,image_info->zero);
>           return(False);
>         }
> +        if ((strpbrk(image_info->filename,ProhibitedAlphabet) != (char *) NULL) ||
> +            (strpbrk(image->filename,ProhibitedAlphabet) != (char *) NULL))
> +          {
> +            ThrowException(exception,FileOpenError,
> +              FilenameContainsProhibitedCharacters,image->filename);
> +            return(False);
> +          }
>         /* Expand sprintf-style codes in delegate command to command string */
>         magick=TranslateText(image_info,image,decode != (char *) NULL ?
>           delegate_info->encode : delegate_info->decode);
> @@ -511,6 +520,13 @@ MagickExport unsigned int InvokeDelegate
>       status=False;
>       goto error_exit;
>     }
> +    if ((strpbrk(image_info->filename,ProhibitedAlphabet) != (char *) NULL) ||
> +        (strpbrk(image->filename,ProhibitedAlphabet) != (char *) NULL))
> +      {
> +        ThrowException(exception,FileOpenError,
> +          FilenameContainsProhibitedCharacters,image->filename);
> +        break;
> +      }
>     /* Expand sprintf-style codes in delegate command to command string */
>     command=TranslateText(image_info,image,commands[i]);
>     if (command == (char *) NULL)
> Index: magick/locale_c.h
> ===================================================================
> RCS file: /GraphicsMagick/GraphicsMagick/magick/locale_c.h,v
> retrieving revision 1.23
> diff -u -3 -p -r1.23 locale_c.h
> --- magick/locale_c.h	17 Feb 2005 05:42:49 -0000	1.23
> +++ magick/locale_c.h	1 Mar 2006 15:53:41 -0000
> @@ -3,7 +3,7 @@
>
> extern MagickExport const char *GetLocaleMessageFromID(const int);
>
> -#define MAX_LOCALE_MSGS 485
> +#define MAX_LOCALE_MSGS 486
>
> #define MGK_BlobErrorUnableToCreateBlob 1
> #define MGK_BlobErrorUnableToOpenFile 2
> @@ -490,6 +490,7 @@ extern MagickExport const char *GetLocal
> #define MGK_XServerFatalErrorUnableToViewFonts 483
> #define MGK_XServerWarningUnableToGetVisual 484
> #define MGK_XServerWarningUsingDefaultVisual 485
> +#define MGK_FileOpenErrorFilenameContainsProhibitedCharacters 486
>
> #endif
>
> @@ -584,7 +585,7 @@ static const SeverityInfo severity_map[]
>     { "XServer/Error", 445, XServerError },
>     { "XServer/FatalError", 467, XServerFatalError },
>     { "XServer/Warning", 483, XServerWarning },
> -    { 0, 485, UndefinedException }
> +    { 0, 486, UndefinedException }
>   };
> #endif
>
> @@ -1082,6 +1083,7 @@ static const MessageInfo message_map[] =
>     { "UnableToViewFonts", 483 },
>     { "UnableToGetVisual", 484 },
>     { "UsingDefaultVisual", 485 },
> +    { "FilenameContainsProhibitedCharacters", 486 },
>     { 0, 0 }
>   };
> #endif
> @@ -1575,6 +1577,7 @@ static const char *message_dat[] =
>     "Unable to view fonts",
>     "Unable to get visual",
>     "UsingDefaultVisual",
> +    "Filename Contains Prohibited Characters",
>     0
>   };
> #endif
>
>
> -------------------------------------------------------
> This SF.Net email is sponsored by xPML, a groundbreaking scripting language
> that extends applications into web and mobile media. Attend the live webcast
> and join the prime developer group breaking into this new coding territory!
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
> _______________________________________________
> Graphicsmagick-core mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/graphicsmagick-core
>

======================================
Bob Friesenhahn
[email protected], http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer,    http://www.GraphicsMagick.org/


-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642