Integer overflows in PNG image loading code #2790

Closed
michaelrsweet opened this Issue Apr 8, 2008 · 3 comments

Comments

Projects
None yet
1 participant
Collaborator

michaelrsweet commented Apr 8, 2008

Version: 1.3-current
CUPS.org User: thomaspollet

1)filter/image-png.c

img->xsize * img->ysize may overflow (CUPS_IMAGE_MAX_WIDTH and CUPS_IMAGE_MAX_HEIGHT are too big for multiplication).

malloc(img->xsize * img->ysize * 3) can result in a buffer that's too small. Also, the return codes of alot of the mallocs aren't checked, when a NULL pointer is passed to png_read_row, it may be possible to corrupt memory this way as well. I have a .png that does this.

2)filter/image-zoom.c
there are a couple of mallocs in this file that look equally dangerous:
malloc(z->xsize * z->depth)
I haven't crashed it but by looking at the code it seems there is no check for the multiplication to overflow (the checks for MAX_WIDTH and MAX_HEIGHT are insufficient when width and height are used in multiplication)

If you need more information, please let me know.

Regards,
Thomas Pollet

Collaborator

michaelrsweet commented Apr 8, 2008

CUPS.org User: mike

Lowering priority to match severity...

The unchecked mallocs will cause the filter to crash but are not a security issue by themselves.

The integer overflow is problematic and will be fixed.

Collaborator

michaelrsweet commented Apr 9, 2008

CUPS.org User: mike

Fixed in Subversion repository.

Collaborator

michaelrsweet commented Apr 9, 2008

"str2790.patch":

Index: image-png.c

--- image-png.c (revision 7434)
+++ image-png.c (working copy)
@@ -3,7 +3,7 @@
*

  • PNG image routines for the Common UNIX Printing System (CUPS).
  • * Copyright 2007 by Apple Inc.
  • * Copyright 2007-2008 by Apple Inc.
  • Copyright 1993-2007 by Easy Software Products.
  • These coded instructions, statements, and computer programs are the
    @@ -170,16 +170,56 @@
  • Interlaced images must be loaded all at once...
    */
  • size_t bufsize; /* Size of buffer */

if (color_type == PNG_COLOR_TYPE_GRAY ||
color_type == PNG_COLOR_TYPE_GRAY_ALPHA)

  •  in = malloc(img->xsize \* img->ysize);
    
  • {
  •  bufsize = img->xsize \* img->ysize;
    
  •  if ((bufsize / img->ysize) != img->xsize)
    
  •  {
    
  • fprintf(stderr, "DEBUG: PNG image dimensions (%ux%u) too large!\n",
  •   (unsigned)width, (unsigned)height);
    
  • fclose(fp);
  • return (1);
  •  }
    
  • }
    else
  •  in = malloc(img->xsize \* img->ysize \* 3);
    
  • {
  •  bufsize = img->xsize \* img->ysize \* 3;
    
  •  if ((bufsize / (img->ysize \* 3)) != img->xsize)
    
  •  {
    
  • fprintf(stderr, "DEBUG: PNG image dimensions (%ux%u) too large!\n",
  •   (unsigned)width, (unsigned)height);
    
  • fclose(fp);
  • return (1);
  •  }
    
  • }
  • in = malloc(bufsize);
    }

bpp = cupsImageGetDepth(img);
out = malloc(img->xsize * bpp);

  • if (!in || !out)
  • {
  • fputs("DEBUG: Unable to allocate memory for PNG image!\n", stderr);
  • if (in)
  •  free(in);
    
  • if (out)
  •  free(out);
    
  • fclose(fp);
  • return (1);
  • }

/*

  • Read the image, interlacing as needed...
    */

michaelrsweet added this to the Stable milestone Mar 17, 2016

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