Fix image handling integer overflows

Image loading memory allocation is based on the image width and height: malloc(heigth * width * 3). Providing an image with large height and width values can cause the result of this calculation to exceed the maximum value of an unsigned int and thus causes an integer overflow. The result: too little memory is allocated and an heap overflow occurs. This patch was based by Niels Heinen <[email protected]> Thanks! Signed-off-by: Nobuhiro Iwamatsu <[email protected]>
axs-gentoo · Aug 30, 2010 · 0e4e4ca · 0e4e4ca
1 parent 8ee8c7b
commit 0e4e4ca
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 80 deletions.
diff --git a/const.h b/const.h
@@ -42,4 +42,7 @@
 // variables replaced in pre-session_cmd and post-session_cmd
 #define USER_VAR       "%user"
 
+// max height/width for images
+#define MAX_DIMENSION 10000
+
 #endif
diff --git a/jpeg.c b/jpeg.c
@@ -22,60 +22,62 @@
 #include <string.h>
 
 #include <jpeglib.h>
+#include "const.h"
 
 int
 read_jpeg(const char *filename, int *width, int *height, unsigned char **rgb)
 {
+    int ret = 0;
     struct jpeg_decompress_struct cinfo;
     struct jpeg_error_mgr jerr;
     unsigned char *ptr = NULL;
     unsigned int i, ipos;
 
     FILE *infile = fopen(filename, "rb");
+    if (infile == NULL) {
+        fprintf(stderr, "Can not fopen file: %s\n",filename);
+        return ret;
+    }
 
     cinfo.err = jpeg_std_error(&jerr);
     jpeg_create_decompress(&cinfo);
     jpeg_stdio_src(&cinfo, infile);
     jpeg_read_header(&cinfo, TRUE);
     jpeg_start_decompress(&cinfo);
 
+    /* Prevent against integer overflow */
+    if(cinfo.output_width >= MAX_DIMENSION || cinfo.output_height >= MAX_DIMENSION) {
+        fprintf(stderr, "Unreasonable dimension found in file: %s\n",filename);
+        goto close_file;
+    }
+
     *width = cinfo.output_width;
     *height = cinfo.output_height;
 
     rgb[0] = malloc(3 * cinfo.output_width * cinfo.output_height);
-    if (rgb[0] == NULL)
-    {
+    if (rgb[0] == NULL) {
         fprintf(stderr, "Can't allocate memory for JPEG file.\n");
-    fclose(infile);
-        return(0);
+        goto close_file;
     }
 
-    if (cinfo.output_components == 3)
-    {
+    if (cinfo.output_components == 3) {
         ptr = rgb[0];
-        while (cinfo.output_scanline < cinfo.output_height) 
-        {
+        while (cinfo.output_scanline < cinfo.output_height) {
             jpeg_read_scanlines(&cinfo, &ptr, 1);
             ptr += 3 * cinfo.output_width;
         }
-    }
-    else if (cinfo.output_components == 1)
-    {
+    } else if (cinfo.output_components == 1) {
         ptr = malloc(cinfo.output_width);
-        if (ptr == NULL)
-        {
+        if (ptr == NULL) {
             fprintf(stderr, "Can't allocate memory for JPEG file.\n");
-        fclose(infile);
-            return(0);
+            goto rgb_free;
         }
 
         ipos = 0;
-        while (cinfo.output_scanline < cinfo.output_height) 
-        {
+        while (cinfo.output_scanline < cinfo.output_height) {
             jpeg_read_scanlines(&cinfo, &ptr, 1);
 
-            for (i = 0; i < cinfo.output_width; i++)
-            {
+            for (i = 0; i < cinfo.output_width; i++) {
                 memset(rgb[0] + ipos, ptr[i], 3);
                 ipos += 3;
             }
@@ -85,9 +87,16 @@ read_jpeg(const char *filename, int *width, int *height, unsigned char **rgb)
     }
 
     jpeg_finish_decompress(&cinfo);
-    jpeg_destroy_decompress(&cinfo);
 
+    ret = 1;
+    goto close_file;
+
+rgb_free:
+    free(rgb[0]);
+
+close_file:
+    jpeg_destroy_decompress(&cinfo);
     fclose(infile);
 
-    return(1);
+    return(ret);
 }
diff --git a/png.c b/png.c
@@ -22,12 +22,13 @@
 #include <stdlib.h>
 
 #include <png.h>
+#include "const.h"
 
 int
 read_png(const char *filename, int *width, int *height, unsigned char **rgb, 
      unsigned char **alpha)
 {
-    FILE *infile = fopen(filename, "rb");
+    int ret = 0;
 
     png_structp png_ptr;
     png_infop info_ptr;
@@ -38,50 +39,51 @@ read_png(const char *filename, int *width, int *height, unsigned char **rgb,
     int bit_depth, color_type, interlace_type;
     int i;
 
+    FILE *infile = fopen(filename, "rb");
+    if (infile == NULL) {
+        fprintf(stderr, "Can not fopen file: %s\n",filename);
+        return ret;
+    }
+
     png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, 
                                      (png_voidp) NULL, 
                                      (png_error_ptr) NULL, 
                                      (png_error_ptr) NULL);
-    if (!png_ptr) 
-    {
-        fclose(infile);
-        return(0);
-    }
+    if (!png_ptr)
+        goto file_close;
 
     info_ptr = png_create_info_struct(png_ptr);
-    if (!info_ptr)
-    {
+    if (!info_ptr) {
         png_destroy_read_struct(&png_ptr, (png_infopp) NULL, 
                                 (png_infopp) NULL);
-        fclose(infile);
-        return(0);
     }
 
     if (setjmp(png_ptr->jmpbuf))
-    {
-        png_destroy_read_struct(&png_ptr, &info_ptr, (png_infopp) NULL);
-        fclose(infile);
-        return(0);
-    }
+        goto png_destroy;
 
     png_init_io(png_ptr, infile);
     png_read_info(png_ptr, info_ptr);
 
     png_get_IHDR(png_ptr, info_ptr, &w, &h, &bit_depth, &color_type,
                  &interlace_type, (int *) NULL, (int *) NULL);
 
+    /* Prevent against integer overflow */
+    if(w >= MAX_DIMENSION || h >= MAX_DIMENSION) {
+        fprintf(stderr, "Unreasonable dimension found in file: %s\n",filename);
+        goto png_destroy;
+    }
+
     *width = (int) w;
     *height = (int) h;
 
     if (color_type == PNG_COLOR_TYPE_RGB_ALPHA
-    || color_type == PNG_COLOR_TYPE_GRAY_ALPHA)
-    {
-    alpha[0] = malloc(*width * *height);
-    if (alpha[0] == NULL)
-    {
-        fprintf(stderr, "Can't allocate memory for alpha channel in PNG file.\n");
-        return(0); 
-    }
+        || color_type == PNG_COLOR_TYPE_GRAY_ALPHA) {
+        alpha[0] = malloc(*width * *height);
+        if (alpha[0] == NULL)
+        {
+            fprintf(stderr, "Can't allocate memory for alpha channel in PNG file.\n");
+            goto png_destroy;
+        }
     }
 
     /* Change a paletted/grayscale image to RGB */
@@ -94,68 +96,70 @@ read_png(const char *filename, int *width, int *height, unsigned char **rgb,
         png_set_gray_to_rgb(png_ptr);
 
     /* If the PNG file has 16 bits per channel, strip them down to 8 */
-    if (bit_depth == 16) png_set_strip_16(png_ptr);
+    if (bit_depth == 16)
+        png_set_strip_16(png_ptr);
 
     /* use 1 byte per pixel */
     png_set_packing(png_ptr);
 
     row_pointers = malloc(*height * sizeof(png_bytep));
-    if (row_pointers == NULL)
-    {
+    if (row_pointers == NULL) {
         fprintf(stderr, "Can't allocate memory for PNG file.\n");
-        return(0);
+        goto png_destroy;
     }
 
-    for (i = 0; i < *height; i++)
-    {
+    for (i = 0; i < *height; i++) {
         row_pointers[i] = malloc(4 * *width);
-        if (row_pointers == NULL)
-        {
+        if (row_pointers == NULL) {
             fprintf(stderr, "Can't allocate memory for PNG line.\n");
-            return(0);
+            goto rows_free;
         }
     }
 
     png_read_image(png_ptr, row_pointers);
 
     rgb[0] = malloc(3 * *width * *height);
-    if (rgb[0] == NULL)
-    {
+    if (rgb[0] == NULL) {
         fprintf(stderr, "Can't allocate memory for PNG file.\n");
-        return(0);
+        goto rows_free;
     }
 
     if (alpha[0] == NULL)
     {
-    ptr = rgb[0];
-    for (i = 0; i < *height; i++)
-    {
-        memcpy(ptr, row_pointers[i], 3 * *width);
-        ptr += 3 * *width;
-    }
-    }
-    else
-    {
-    int j;
-    ptr = rgb[0];
-    for (i = 0; i < *height; i++)
-    {
-        int ipos = 0;
-        for (j = 0; j < *width; j++)
-        {
-        *ptr++ = row_pointers[i][ipos++];
-        *ptr++ = row_pointers[i][ipos++];
-        *ptr++ = row_pointers[i][ipos++];
-        alpha[0][i * *width + j] = row_pointers[i][ipos++];
+        ptr = rgb[0];
+        for (i = 0; i < *height; i++) {
+            memcpy(ptr, row_pointers[i], 3 * *width);
+            ptr += 3 * *width;
+        }
+    } else {
+        int j;
+        ptr = rgb[0];
+        for (i = 0; i < *height; i++) {
+            int ipos = 0;
+            for (j = 0; j < *width; j++) {
+                *ptr++ = row_pointers[i][ipos++];
+                *ptr++ = row_pointers[i][ipos++];
+                *ptr++ = row_pointers[i][ipos++];
+                alpha[0][i * *width + j] = row_pointers[i][ipos++];
+            }
         }
-    }
     }
 
-    png_destroy_read_struct(&png_ptr, &info_ptr, (png_infopp) NULL);
+    ret = 1; /* data reading is OK */
+
+rows_free:
+    for (i = 0; i < *height; i++) {
+        if (row_pointers[i] != NULL ) {
+            free(row_pointers[i]);
+        }
+    }
 
-    for (i = 0; i < *height; i++) free(row_pointers[i]);
     free(row_pointers);
 
+png_destroy:
+    png_destroy_read_struct(&png_ptr, &info_ptr, (png_infopp) NULL);
+
+file_close:
     fclose(infile);
-    return(1);
+    return(ret);
 }