From d5ebec2948d93c0c47c249e1506a1a6bdbf27b68 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Sat, 11 Mar 2017 11:39:13 +0100 Subject: [PATCH] Properly release resources on error path The code did not properly release resources in some error paths, leading to memory leaks or possible double free issues. If an image could not be loaded, some code paths check if width is 0 to determine if an error occurred. Therefore, always set width to 0 in such cases. --- src/modules/loaders/loader_argb.c | 9 +++++++-- src/modules/loaders/loader_bmp.c | 3 +++ src/modules/loaders/loader_ff.c | 4 ++++ src/modules/loaders/loader_gif.c | 6 ++++++ src/modules/loaders/loader_png.c | 3 +++ src/modules/loaders/loader_pnm.c | 6 ++++++ src/modules/loaders/loader_tga.c | 12 +++++++++++- src/modules/loaders/loader_tiff.c | 15 +++++++++------ src/modules/loaders/loader_xpm.c | 9 +++++++++ 9 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/modules/loaders/loader_argb.c b/src/modules/loaders/loader_argb.c index 57f3dcf..4c1bc81 100644 --- a/src/modules/loaders/loader_argb.c +++ b/src/modules/loaders/loader_argb.c @@ -62,6 +62,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, ptr = im->data = malloc(w * h * sizeof(DATA32)); if (!im->data) { + im->w = 0; fclose(f); return 0; } @@ -73,7 +74,9 @@ load(ImlibImage * im, ImlibProgressFunction progress, if (fread(ptr, im->w, 4, f) != 4) { - free(ptr); + free(im->data); + im->data = NULL; + im->w = 0; fclose(f); return 0; } @@ -83,7 +86,9 @@ load(ImlibImage * im, ImlibProgressFunction progress, #else if (fread(ptr, im->w, 4, f) != 4) { - free(ptr); + free(im->data); + im->data = NULL; + im->w = 0; fclose(f); return 0; } diff --git a/src/modules/loaders/loader_bmp.c b/src/modules/loaders/loader_bmp.c index 5b9730b..2d15ba5 100644 --- a/src/modules/loaders/loader_bmp.c +++ b/src/modules/loaders/loader_bmp.c @@ -301,6 +301,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, im->data = malloc(w * h * sizeof(DATA32)); if (!im->data) { + im->w = 0; free(buffer); fclose(f); return 0; @@ -309,6 +310,8 @@ load(ImlibImage * im, ImlibProgressFunction progress, if (fread(buffer, imgsize, 1, f) != 1) { free(im->data); + im->data = NULL; + im->w = 0; free(buffer); fclose(f); return 0; diff --git a/src/modules/loaders/loader_ff.c b/src/modules/loaders/loader_ff.c index bd98eae..267a9cb 100644 --- a/src/modules/loaders/loader_ff.c +++ b/src/modules/loaders/loader_ff.c @@ -35,6 +35,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, im->h = ntohl(hdr[3]); if (!IMAGE_DIMENSIONS_OK(im->w, im->h)) { + im->w = 0; fclose(f); return 0; } @@ -44,6 +45,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, { if (!(im->format = strdup("ff"))) { + im->w = 0; fclose(f); return 0; } @@ -64,6 +66,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, { free(im->data); im->data = NULL; + im->w = 0; free(row); fclose(f); return 0; @@ -76,6 +79,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, { free(im->data); im->data = NULL; + im->w = 0; free(row); fclose(f); return 0; diff --git a/src/modules/loaders/loader_gif.c b/src/modules/loaders/loader_gif.c index 9672d82..036ee93 100644 --- a/src/modules/loaders/loader_gif.c +++ b/src/modules/loaders/loader_gif.c @@ -204,6 +204,12 @@ load(ImlibImage * im, ImlibProgressFunction progress, char progress_granularity, DGifCloseFile(gif); #endif + if (rc == 0) + { + free(im->data); + im->data = NULL; + im->w = 0; + } return rc; } diff --git a/src/modules/loaders/loader_png.c b/src/modules/loaders/loader_png.c index 31f81b2..7a488b5 100644 --- a/src/modules/loaders/loader_png.c +++ b/src/modules/loaders/loader_png.c @@ -82,6 +82,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, png_read_end(png_ptr, info_ptr); png_destroy_read_struct(&png_ptr, &info_ptr, (png_infopp) NULL); fclose(f); + im->w = 0; return 0; } if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) @@ -151,6 +152,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, im->data = malloc(w * h * sizeof(DATA32)); if (!im->data) { + im->w = 0; png_read_end(png_ptr, info_ptr); png_destroy_read_struct(&png_ptr, &info_ptr, (png_infopp) NULL); fclose(f); @@ -162,6 +164,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, { free(im->data); im->data = NULL; + im->w = 0; png_read_end(png_ptr, info_ptr); png_destroy_read_struct(&png_ptr, &info_ptr, (png_infopp) NULL); fclose(f); diff --git a/src/modules/loaders/loader_pnm.c b/src/modules/loaders/loader_pnm.c index 04b01b0..8076289 100644 --- a/src/modules/loaders/loader_pnm.c +++ b/src/modules/loaders/loader_pnm.c @@ -539,6 +539,12 @@ load(ImlibImage * im, ImlibProgressFunction progress, } quit: fclose(f); + if (rc == 0) + { + free(im->data); + im->data = NULL; + im->w = 0; + } return rc; } diff --git a/src/modules/loaders/loader_tga.c b/src/modules/loaders/loader_tga.c index 68274d5..c115741 100644 --- a/src/modules/loaders/loader_tga.c +++ b/src/modules/loaders/loader_tga.c @@ -292,6 +292,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, if (!IMAGE_DIMENSIONS_OK(im->w, im->h)) { munmap(seg, ss.st_size); + im->w = 0; close(fd); return 0; } @@ -318,8 +319,8 @@ load(ImlibImage * im, ImlibProgressFunction progress, im->data = malloc(im->w * im->h * sizeof(DATA32)); if (!im->data) { - im->w = 0; munmap(seg, ss.st_size); + im->w = 0; close(fd); return 0; } @@ -361,6 +362,9 @@ load(ImlibImage * im, ImlibProgressFunction progress, if (bufptr + bpp / 8 > bufend) { munmap(seg, ss.st_size); + free(im->data); + im->data = NULL; + im->w = 0; close(fd); return 0; } @@ -420,6 +424,9 @@ load(ImlibImage * im, ImlibProgressFunction progress, if ((bufptr + 1 + (bpp / 8)) > bufend) { munmap(seg, ss.st_size); + free(im->data); + im->data = NULL; + im->w = 0; close(fd); return 0; } @@ -482,6 +489,9 @@ load(ImlibImage * im, ImlibProgressFunction progress, if ((bufptr + 1 + (bpp / 8)) > bufend) { munmap(seg, ss.st_size); + free(im->data); + im->data = NULL; + im->w = 0; close(fd); return 0; } diff --git a/src/modules/loaders/loader_tiff.c b/src/modules/loaders/loader_tiff.c index 8f340d6..f9c771a 100644 --- a/src/modules/loaders/loader_tiff.c +++ b/src/modules/loaders/loader_tiff.c @@ -329,7 +329,10 @@ load(ImlibImage * im, ImlibProgressFunction progress, break; } if (!IMAGE_DIMENSIONS_OK(im->w, im->h)) - goto quit2; + { + im->w = 0; + goto quit2; + } rgba_image.num_pixels = num_pixels = im->w * im->h; if (rgba_image.rgba.alpha != EXTRASAMPLE_UNSPECIFIED) SET_FLAG(im->flags, F_HAS_ALPHA); @@ -352,11 +355,9 @@ load(ImlibImage * im, ImlibProgressFunction progress, if (rast) _TIFFfree(rast); - if (im->data) - { - free(im->data); - im->data = NULL; - } + free(im->data); + im->data = NULL; + im->w = 0; goto quit2; } @@ -367,6 +368,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, _TIFFfree(rast); free(im->data); im->data = NULL; + im->w = 0; goto quit2; } @@ -387,6 +389,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, _TIFFfree(rast); free(im->data); im->data = NULL; + im->w = 0; goto quit2; } diff --git a/src/modules/loaders/loader_xpm.c b/src/modules/loaders/loader_xpm.c index 412a1bd..a7a561b 100644 --- a/src/modules/loaders/loader_xpm.c +++ b/src/modules/loaders/loader_xpm.c @@ -227,6 +227,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, char progress_granularity, if (!cmap) { + im->w = 0; free(line); fclose(f); xpm_parse_done(); @@ -241,6 +242,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, char progress_granularity, (DATA32 *) malloc(sizeof(DATA32) * im->w * im->h); if (!im->data) { + im->w = 0; free(cmap); free(line); fclose(f); @@ -253,6 +255,7 @@ load(ImlibImage * im, ImlibProgressFunction progress, char progress_granularity, } else { + im->w = 0; free(cmap); free(line); fclose(f); @@ -279,6 +282,9 @@ load(ImlibImage * im, ImlibProgressFunction progress, char progress_granularity, len = strlen(line); if (len < cpp) { + free(im->data); + im->data = NULL; + im->w = 0; free(cmap); free(line); fclose(f); @@ -521,6 +527,9 @@ load(ImlibImage * im, ImlibProgressFunction progress, char progress_granularity, nline = realloc(line, lsz); if (nline == NULL) { + free(im->data); + im->data = NULL; + im->w = 0; free(cmap); free(line); fclose(f);