Browse Source

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 <niels@freebsd.org>
Thanks!

Signed-off-by: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>

git-svn-id: svn+ssh://svn.berlios.de/svnroot/repos/slim/trunk@176 7c53e7cc-98ea-0310-8f1f-a0b24da60408
iwamatsu 13 years ago
parent
commit
230709cab5
3 changed files with 96 additions and 80 deletions
  1. 3 0
      const.h
  2. 30 21
      jpeg.c
  3. 63 59
      png.c

+ 3 - 0
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

+ 30 - 21
jpeg.c

@@ -22,16 +22,22 @@
 #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);
@@ -39,43 +45,39 @@ read_jpeg(const char *filename, int *width, int *height, unsigned char **rgb)
     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);
 }

+ 63 - 59
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,31 +39,27 @@ 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);
@@ -70,18 +67,23 @@ read_png(const char *filename, int *width, int *height, unsigned char **rgb,
     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);
 }