Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential Memory Leak in ReadSIXELImage in coders/sixel.c #1452

Closed
3 tasks done
twelveand0 opened this issue Jan 17, 2019 · 2 comments
Closed
3 tasks done

Potential Memory Leak in ReadSIXELImage in coders/sixel.c #1452

twelveand0 opened this issue Jan 17, 2019 · 2 comments
Labels
Milestone

Comments

@twelveand0
Copy link

Prerequisites

  • I have written a descriptive issue title
  • I have verified that I am using the latest version of ImageMagick
  • I have searched open and closed issues to ensure it has not already been reported

Description

potential memory leak in ReadSIXELImage in sixel.c

Steps to Reproduce

The first critical code snippet is:
https://github.com/ImageMagick/ImageMagick/blob/master/coders/sixel.c#L539

if (imsx > max_x || imsy > max_y) { // line 539
        dmsx = max_x;
        dmsy = max_y;
        if (SetImageExtent(image,dmsx,dmsy,exception) == MagickFalse)
          {
            imbuf = (unsigned char *) RelinquishMagickMemory(imbuf);
            return (MagickFalse);
          }
        if ((dmbuf = (unsigned char *) AcquireQuantumMemory(dmsx , dmsy)) == NULL) { //line 547
            imbuf = (unsigned char *) RelinquishMagickMemory(imbuf);
            return (MagickFalse);
        }
        for (y = 0; y < dmsy; ++y) {
            (void) memcpy(dmbuf + dmsx * y, imbuf + imsx * y, dmsx);
        }
        imbuf = (unsigned char *) RelinquishMagickMemory(imbuf);
        imsx = dmsx;
        imsy = dmsy;
        imbuf = dmbuf;  //line 557
    }

    *pixels = imbuf;  //line 560
    *pwidth = imsx;
    *pheight = imsy;
    *ncolors = max_color_index + 1;
    *palette = (unsigned char *) AcquireQuantumMemory(*ncolors,4); // line 564
    if (*palette == (unsigned char *) NULL)
      return(MagickFalse);     // line 566

When condition at line 539 is satisfied and dmbuf is successfully allocated at line 547, the value of dmbuf is assigned to imbuf at line 557 and is finally assigned to the pointer parameter pixels at line 560 (i.e. the buf's address is passed outside to the caller function).

Now, when the allocation at line 564 failed, the function will return MagickFalse at line 566.

Next, I searched the whole project code and only found one call to sixel_decode which locates in function ReadSIXELImage in sixel.c at line 1057 as the following. The local variable sixel_pixels holds the value of dmbuf.
https://github.com/ImageMagick/ImageMagick/blob/master/coders/sixel.c#L1057

if (sixel_decode(image,(unsigned char *) sixel_buffer,&sixel_pixels,&image->columns,&image->rows,&sixel_palette,&image->colors,exception) == MagickFalse)  // line 1057
    {
      sixel_buffer=(char *) RelinquishMagickMemory(sixel_buffer);
      ThrowReaderException(CorruptImageError,"CorruptImage");
    }
  sixel_buffer=(char *) RelinquishMagickMemory(sixel_buffer);
  image->depth=24;
  image->storage_class=PseudoClass;
  status=SetImageExtent(image,image->columns,image->rows,exception);
  if (status == MagickFalse)
    {
      sixel_pixels=(unsigned char *) RelinquishMagickMemory(sixel_pixels);  // line 1068
      sixel_palette=(unsigned char *) RelinquishMagickMemory(sixel_palette);
      return(DestroyImageList(image));
    }

However, when function sixel_decode returned MagickFalse as described above, the memory pointed by sixel_pixels (i.e. the memory allocated at line 547) was not freed as done at line 1068. As a result, a memory leak happens.

Patch Suggestion:

if (sixel_decode(image,(unsigned char *) sixel_buffer,&sixel_pixels,&image->columns,&image->rows,&sixel_palette,&image->colors,exception) == MagickFalse)
    {
      sixel_buffer=(char *) RelinquishMagickMemory(sixel_buffer);
+     sixel_pixels=(unsigned char *) RelinquishMagickMemory(sixel_pixels);
      ThrowReaderException(CorruptImageError,"CorruptImage");
    }

System Configuration

  • ImageMagick version: ImageMagick-4f0ea40e2a090e245f31d1f05247520d6e7eb4ca
  • Environment (Operating system, version and so on): Ubuntu 16.04
  • Additional information:

Credit to Bingchang Liu at VARAS of IIE

@urban-warrior
Copy link
Member

Thanks for the problem report. We can reproduce it and will have a patch to fix it in GIT master branch @ https://github.com/ImageMagick/ImageMagick later today. The patch will be available in the beta releases of ImageMagick @ https://www.imagemagick.org/download/beta/ by sometime tomorrow.

urban-warrior pushed a commit to ImageMagick/ImageMagick6 that referenced this issue Jan 20, 2019
@dlemstra dlemstra added the bug label Jan 20, 2019
@dlemstra dlemstra added this to the 7.0.8-25 milestone Jan 20, 2019
@nohmask
Copy link

nohmask commented Feb 12, 2019

This was assigned CVE-2019-7396.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants