Comment 9 for bug 27767

Message-ID: <email address hidden>
Date: Thu, 5 Jan 2006 13:49:11 +0100
From: Daniel Kobras <email address hidden>
To: Florian Weimer <email address hidden>, <email address hidden>
Subject: Re: Bug#345238: Shell command injection in delegate code (via file names)

--7JfCtLOvnd9MIVvH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

tag 345238 + patch
thanks

On Fri, Dec 30, 2005 at 02:19:27PM +0100, Florian Weimer wrote:
> With some user interaction, this is exploitable through Gnus and
> Thunderbird. I think this warrants increasing the severity to
> "grave".

Here's the vanilla fix from upstream SVN, stripped off whitespace changes.
I wonder why they've banned ` but still allow $(...), though.

Regards,

Daniel.

--7JfCtLOvnd9MIVvH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline; filename="CVE-2005-4601.diff"

--- delegate.c.orig 2006-01-05 13:37:47.000000000 +0100
+++ delegate.c 2006-01-05 13:45:00.000000000 +0100
@@ -701,6 +701,8 @@
 MagickExport MagickBooleanType InvokeDelegate(ImageInfo *image_info,
   Image *image,const char *decode,const char *encode,ExceptionInfo *exception)
 {
+#define ProhibitedAlphabet "*?\"'<>|`"
+
   char
     *command,
     **commands;
@@ -753,11 +755,11 @@
         }
       image_info->temporary=MagickTrue;
     }
- if (delegate_info->mode != 0)
- if (((decode != (const char *) NULL) &&
+ if ((delegate_info->mode != 0) &&
+ (((decode != (const char *) NULL) &&
          (delegate_info->encode != (char *) NULL)) ||
         ((encode != (const char *) NULL) &&
- (delegate_info->decode != (char *) NULL)))
+ (delegate_info->decode != (char *) NULL))))
       {
         char
           *magick;
@@ -771,6 +773,13 @@
         /*
           Delegate requires a particular image format.
         */
+ if ((strpbrk(image_info->filename,ProhibitedAlphabet) != (char *) NULL) ||
+ (strpbrk(image->filename,ProhibitedAlphabet) != (char *) NULL))
+ {
+ ThrowFileException(exception,FileOpenError,
+ "FilenameContainsProhibitedCharacters",image->filename);
+ return(MagickFalse);
+ }
         if (AcquireUniqueFilename(image_info->unique) == MagickFalse)
           {
             ThrowFileException(exception,FileOpenError,
@@ -850,18 +859,25 @@
   for (i=0; commands[i] != (char *) NULL; i++)
   {
     status=MagickFalse;
+ if ((strpbrk(image_info->filename,ProhibitedAlphabet) != (char *) NULL) ||
+ (strpbrk(image->filename,ProhibitedAlphabet) != (char *) NULL))
+ {
+ ThrowFileException(exception,FileOpenError,
+ "FilenameContainsProhibitedCharacters",image->filename);
+ break;
+ }
     if (AcquireUniqueFilename(image_info->unique) == MagickFalse)
       {
         ThrowFileException(exception,FileOpenError,
           "UnableToCreateTemporaryFile",image_info->unique);
- return(MagickFalse);
+ break;
       }
     if (AcquireUniqueFilename(image_info->zero) == MagickFalse)
       {
         (void) RelinquishUniqueFileResource(image_info->unique);
         ThrowFileException(exception,FileOpenError,
           "UnableToCreateTemporaryFile",image_info->zero);
- return(MagickFalse);
+ break;
       }
     command=TranslateText(image_info,image,commands[i]);
     if (command == (char *) NULL)

--7JfCtLOvnd9MIVvH--