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

memory leak in DecodeImage in coders/pcd.c different from #1193 and #811 #1450

Closed
twelveand0 opened this Issue Jan 17, 2019 · 3 comments

Comments

Projects
None yet
4 participants
@twelveand0
Copy link

twelveand0 commented Jan 17, 2019

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

memory leak in DecodeImage in coders/pcd.c, which is different from #1193 and #811

Steps to Reproduce

The critical code snippet is:
https://github.com/ImageMagick/ImageMagick/blob/master/coders/pcd.c#L198

for (i=0; i < (image->columns > 1536 ? 3 : 1); i++)
  {
    PCDGetBits(8);
    length=(sum & 0xff)+1;
    pcd_table[i]=(PCDTable *) AcquireQuantumMemory(length,  //line 202
      sizeof(*pcd_table[i]));
    if (pcd_table[i] == (PCDTable *) NULL)              //line 204
      {
        buffer=(unsigned char *) RelinquishMagickMemory(buffer); 
        ThrowBinaryException(ResourceLimitError,"MemoryAllocationFailed",  //line 207
          image->filename);
      }
    r=pcd_table[i];
    for (j=0; j < (ssize_t) length; j++)
    {
      PCDGetBits(8);
      r->length=(unsigned int) (sum & 0xff)+1;
      if (r->length > 16)                                       //line 215
        {
          buffer=(unsigned char *) RelinquishMagickMemory(buffer);
          return(MagickFalse);                            //line 218
        }
      PCDGetBits(16);
      r->sequence=(unsigned int) (sum & 0xffff) << 16;
      PCDGetBits(8);
      r->key=(unsigned char) (sum & 0xff);
      r->mask=(~((1U << (32-r->length))-1));
      r++;
    }
    pcd_length[i]=(size_t) length;
  }

pcd_table[i] is allocated at line 202, however, pcd_table[0~i] are forgot to be freed when exception happens at line 204 or function returns at line 215. pcd_table is a local array variable and it isn't passed to the caller function when DecodeImage returns with MagickFalse at line 207 and 218. As a result, the allocated memory pcd_table[0~(i-1)] (or pcd_table[0~i] when returned at line 218) will not be freed and memory leak happens.

The max size of leaked memory is 3*(0xff+1)*sizeof(*pcd_table[0])

The patch suggestion:

if (pcd_table[i] == (PCDTable *) NULL)    //line 204
      {
        buffer=(unsigned char *) RelinquishMagickMemory(buffer);
        for (k=0; k < i; k++)
          pcd_table[k] = (PCDTable *)RelinquishMagickMemory(pcd_table[k]);
        ThrowBinaryException(ResourceLimitError,"MemoryAllocationFailed",
          image->filename);
      }

if (r->length > 16)
        {
          buffer=(unsigned char *) RelinquishMagickMemory(buffer);
          for (k=0; k <= i; k++)
            pcd_table[k] = (PCDTable *)RelinquishMagickMemory(pcd_table[k]);
          return(MagickFalse);
        }

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 pushed a commit that referenced this issue Jan 20, 2019

Cristy

urban-warrior pushed a commit to ImageMagick/ImageMagick6 that referenced this issue Jan 20, 2019

Cristy
@urban-warrior

This comment has been minimized.

Copy link
Contributor

urban-warrior commented Jan 20, 2019

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.

@dlemstra dlemstra added the bug label Jan 20, 2019

@dlemstra dlemstra added this to the 7.0.8-25 milestone Jan 20, 2019

@twelveand0

This comment has been minimized.

Copy link
Author

twelveand0 commented Jan 21, 2019

Thanks for your work.

@dlemstra dlemstra closed this Jan 28, 2019

@nohmask

This comment has been minimized.

Copy link

nohmask commented Mar 11, 2019

This was assigned CVE-2019-7175.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.