SegFault in url.c when creating a temp file fails

[warhammerkid]

In the below code, image is initialized to NULL. If AcquireUniqueFileResource or fdopen fails, the code tries to copy the filename to image->filename, which causes a SegFault because the destination is invalid. I ran into this because the system tmp directory was not writable by the user I was running this under (and it took me forever to figure this out because I figured RMagick was to blame).

Code: Select all

  //url.c:147
  image=(Image *) NULL;
  read_info=CloneImageInfo(image_info);
  SetImageInfoBlob(read_info,(void *) NULL,0);
  file=(FILE *) NULL;
  unique_file=AcquireUniqueFileResource(read_info->filename);
  if (unique_file != -1)
    file=fdopen(unique_file,"wb");
  if ((unique_file == -1) || (file == (FILE *) NULL))
    {
      read_info=DestroyImageInfo(read_info);
      (void) CopyMagickString(image->filename,read_info->filename,
        MaxTextExtent);

[magick]

We can reproduce the problem you posted and have a patch in ImageMagick 6.8.4-7 Beta available by sometime tomorrow. Thanks.

[broucaries]

What are the platform (windows ? Unix?) affected ?

What is the symptom ? (Null pointer dereference ? )

What is the first version affected ?

Bastien

[magick]

This bug has likely been around for quite some time (e.g. years). It only occurs when you use a URL as an image filename and you can't write to the temporary directory which is typically /tmp or whereever MAGICK_TMPDIR points. That triggers an exception message that references read_info->filename after its already been freed.

[broucaries]

Thanks so:
- every plateform is affected
- it is trigerable by user (use MAGICK_TMPDIR=nonexsitant)
- it trigger a NULL deference

So you should post here a minimal patch.

I will ask a CVE for the debian side.

[magick]

Code: Select all

-> diff coders~/url.c coders/url.c 
157,159d156
<       read_info=DestroyImageInfo(read_info);
<       (void) CopyMagickString(image->filename,read_info->filename,
<         MaxTextExtent);
161,162c158,159
<         image->filename);
<       image=DestroyImageList(image);
---
>         read_info->filename);
>       read_info=DestroyImageInfo(read_info);