From b2eed83c034c333cf5b6b4f63d731da4c55aaa56 Mon Sep 17 00:00:00 2001
From: Ozkan Sezer <[EMAIL REDACTED]>
Date: Wed, 9 Apr 2025 07:23:32 +0300
Subject: [PATCH] Factor out parts of IMG_LoadJPG where locals can be clobbered
by longjmp
Also sanitize error setting a bit in IMG_LoadPNG()
Fixes https://github.com/libsdl-org/SDL_image/issues/435
---
src/IMG_jpg.c | 134 ++++++++++++++++++++++++++++----------------------
src/IMG_png.c | 49 +++++++++---------
2 files changed, 101 insertions(+), 82 deletions(-)
diff --git a/src/IMG_jpg.c b/src/IMG_jpg.c
index 9abc76e21..3f650a44d 100644
--- a/src/IMG_jpg.c
+++ b/src/IMG_jpg.c
@@ -124,6 +124,7 @@ static bool IMG_InitJPG(void)
return true;
}
+
#if 0
void IMG_QuitJPG(void)
{
@@ -343,89 +344,106 @@ static void output_no_message(j_common_ptr cinfo)
(void)cinfo;
}
-/* Load a JPEG type image from an SDL datasource */
-SDL_Surface *IMG_LoadJPG_IO(SDL_IOStream *src)
-{
- Sint64 start;
+struct loadjpeg_vars {
+ const char *error;
+ SDL_Surface *surface;
struct jpeg_decompress_struct cinfo;
- JSAMPROW rowptr[1];
- SDL_Surface *surface = NULL;
struct my_error_mgr jerr;
+};
- if ( !src ) {
- /* The error message has been set in SDL_IOFromFile */
- return NULL;
- }
- start = SDL_TellIO(src);
-
- if (!IMG_InitJPG()) {
- return NULL;
- }
+/* Load a JPEG type image from an SDL datasource */
+static bool LIBJPEG_LoadJPG_IO(SDL_IOStream *src, struct loadjpeg_vars *vars)
+{
+ JSAMPROW rowptr[1];
/* Create a decompression structure and load the JPEG header */
- cinfo.err = lib.jpeg_std_error(&jerr.errmgr);
- jerr.errmgr.error_exit = my_error_exit;
- jerr.errmgr.output_message = output_no_message;
-#ifdef _MSC_VER
-#pragma warning(disable:4611) /* warning C4611: interaction between '_setjmp' and C++ object destruction is non-portable */
-#endif
- if (setjmp(jerr.escape)) {
+ vars->cinfo.err = lib.jpeg_std_error(&vars->jerr.errmgr);
+ vars->jerr.errmgr.error_exit = my_error_exit;
+ vars->jerr.errmgr.output_message = output_no_message;
+ if (setjmp(vars->jerr.escape)) {
/* If we get here, libjpeg found an error */
- lib.jpeg_destroy_decompress(&cinfo);
- if ( surface != NULL ) {
- SDL_DestroySurface(surface);
- }
- SDL_SeekIO(src, start, SDL_IO_SEEK_SET);
- SDL_SetError("JPEG loading error");
- return NULL;
+ lib.jpeg_destroy_decompress(&vars->cinfo);
+ vars->error = "JPEG loading error";
+ return false;
}
- lib.jpeg_create_decompress(&cinfo);
- jpeg_SDL_IO_src(&cinfo, src);
- lib.jpeg_read_header(&cinfo, TRUE);
+ lib.jpeg_create_decompress(&vars->cinfo);
+ jpeg_SDL_IO_src(&vars->cinfo, src);
+ lib.jpeg_read_header(&vars->cinfo, TRUE);
- if (cinfo.num_components == 4) {
+ if (vars->cinfo.num_components == 4) {
/* Set 32-bit Raw output */
- cinfo.out_color_space = JCS_CMYK;
- cinfo.quantize_colors = FALSE;
- lib.jpeg_calc_output_dimensions(&cinfo);
+ vars->cinfo.out_color_space = JCS_CMYK;
+ vars->cinfo.quantize_colors = FALSE;
+ lib.jpeg_calc_output_dimensions(&vars->cinfo);
/* Allocate an output surface to hold the image */
- surface = SDL_CreateSurface(cinfo.output_width, cinfo.output_height, SDL_PIXELFORMAT_BGRA32);
+ vars->surface = SDL_CreateSurface(vars->cinfo.output_width, vars->cinfo.output_height, SDL_PIXELFORMAT_BGRA32);
} else {
/* Set 24-bit RGB output */
- cinfo.out_color_space = JCS_RGB;
- cinfo.quantize_colors = FALSE;
+ vars->cinfo.out_color_space = JCS_RGB;
+ vars->cinfo.quantize_colors = FALSE;
#ifdef FAST_JPEG
- cinfo.scale_num = 1;
- cinfo.scale_denom = 1;
- cinfo.dct_method = JDCT_FASTEST;
- cinfo.do_fancy_upsampling = FALSE;
+ vars->cinfo.scale_num = 1;
+ vars->cinfo.scale_denom = 1;
+ vars->cinfo.dct_method = JDCT_FASTEST;
+ vars->cinfo.do_fancy_upsampling = FALSE;
#endif
- lib.jpeg_calc_output_dimensions(&cinfo);
+ lib.jpeg_calc_output_dimensions(&vars->cinfo);
/* Allocate an output surface to hold the image */
- surface = SDL_CreateSurface(cinfo.output_width, cinfo.output_height, SDL_PIXELFORMAT_RGB24);
+ vars->surface = SDL_CreateSurface(vars->cinfo.output_width, vars->cinfo.output_height, SDL_PIXELFORMAT_RGB24);
}
- if ( surface == NULL ) {
- lib.jpeg_destroy_decompress(&cinfo);
- SDL_SeekIO(src, start, SDL_IO_SEEK_SET);
- SDL_SetError("Out of memory");
- return NULL;
+ if (!vars->surface) {
+ lib.jpeg_destroy_decompress(&vars->cinfo);
+ return false;
}
/* Decompress the image */
- lib.jpeg_start_decompress(&cinfo);
- while ( cinfo.output_scanline < cinfo.output_height ) {
- rowptr[0] = (JSAMPROW)(Uint8 *)surface->pixels +
- cinfo.output_scanline * surface->pitch;
- lib.jpeg_read_scanlines(&cinfo, rowptr, (JDIMENSION) 1);
+ lib.jpeg_start_decompress(&vars->cinfo);
+ while (vars->cinfo.output_scanline < vars->cinfo.output_height) {
+ rowptr[0] = (JSAMPROW)(Uint8 *)vars->surface->pixels +
+ vars->cinfo.output_scanline * vars->surface->pitch;
+ lib.jpeg_read_scanlines(&vars->cinfo, rowptr, (JDIMENSION) 1);
+ }
+ lib.jpeg_finish_decompress(&vars->cinfo);
+ lib.jpeg_destroy_decompress(&vars->cinfo);
+
+ return true;
+}
+
+SDL_Surface *IMG_LoadJPG_IO(SDL_IOStream *src)
+{
+ Sint64 start;
+ struct loadjpeg_vars vars;
+
+ if (!src) {
+ /* The error message has been set in SDL_IOFromFile */
+ return NULL;
}
- lib.jpeg_finish_decompress(&cinfo);
- lib.jpeg_destroy_decompress(&cinfo);
- return surface;
+ if (!IMG_InitJPG()) {
+ return NULL;
+ }
+
+ start = SDL_TellIO(src);
+ SDL_zero(vars);
+
+ if (LIBJPEG_LoadJPG_IO(src, &vars)) {
+ return vars.surface;
+ }
+
+ /* this may clobber a set error if seek fails: don't care. */
+ SDL_SeekIO(src, start, SDL_IO_SEEK_SET);
+ if (vars.surface) {
+ SDL_DestroySurface(vars.surface);
+ }
+ if (vars.error) {
+ SDL_SetError("%s", vars.error);
+ }
+
+ return NULL;
}
#define OUTPUT_BUFFER_SIZE 4096
diff --git a/src/IMG_png.c b/src/IMG_png.c
index f0adb0199..ac6b58100 100644
--- a/src/IMG_png.c
+++ b/src/IMG_png.c
@@ -186,6 +186,7 @@ static bool IMG_InitPNG(void)
return true;
}
+
#if 0
void IMG_QuitPNG(void)
{
@@ -243,7 +244,7 @@ struct loadpng_vars {
png_bytep *row_pointers;
};
-static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
+static bool LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
{
png_uint_32 width, height;
int bit_depth, color_type, interlace_type, num_channels;
@@ -257,14 +258,14 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
NULL,NULL,NULL);
if (vars->png_ptr == NULL) {
vars->error = "Couldn't allocate memory for PNG file or incompatible PNG dll";
- return;
+ return false;
}
/* Allocate/initialize the memory for image information. REQUIRED. */
vars->info_ptr = lib.png_create_info_struct(vars->png_ptr);
if (vars->info_ptr == NULL) {
vars->error = "Couldn't create image information for PNG file";
- return;
+ return false;
}
/* Set error handling if you are using setjmp/longjmp method (this is
@@ -273,9 +274,6 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
*/
#ifdef PNG_SETJMP_SUPPORTED
-#ifdef _MSC_VER
-#pragma warning(disable:4611) /* warning C4611: interaction between '_setjmp' and C++ object destruction is non-portable */
-#endif
#ifndef LIBPNG_VERSION_12
if (setjmp(*lib.png_set_longjmp_fn(vars->png_ptr, longjmp, sizeof(jmp_buf))))
#else
@@ -283,7 +281,7 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
#endif
{
vars->error = "Error reading the PNG file.";
- return;
+ return false;
}
#endif
/* Set up the input control */
@@ -390,8 +388,7 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
vars->surface = SDL_CreateSurface(width, height, format);
if (vars->surface == NULL) {
- vars->error = SDL_GetError();
- return;
+ return false;
}
if (ckey != -1) {
@@ -409,7 +406,7 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
vars->row_pointers = (png_bytep*) SDL_malloc(sizeof(png_bytep)*height);
if (!vars->row_pointers) {
vars->error = "Out of memory";
- return;
+ return false;
}
for (row = 0; row < (int)height; row++) {
vars->row_pointers[row] = (png_bytep)
@@ -429,15 +426,14 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
/* Load the palette, if any */
if (SDL_ISPIXELFORMAT_INDEXED(vars->surface->format)) {
- SDL_Palette *palette = NULL;
+ SDL_Palette *palette;
int png_num_palette;
png_colorp png_palette;
lib.png_get_PLTE(vars->png_ptr, vars->info_ptr, &png_palette, &png_num_palette);
if (color_type == PNG_COLOR_TYPE_GRAY) {
palette = SDL_CreateSurfacePalette(vars->surface);
if (!palette) {
- vars->error = SDL_GetError();
- return;
+ return false;
}
for (i = 0; i < 256; i++) {
palette->colors[i].r = (Uint8)i;
@@ -447,8 +443,7 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
} else if (png_num_palette > 0 ) {
palette = SDL_CreateSurfacePalette(vars->surface);
if (!palette) {
- vars->error = SDL_GetError();
- return;
+ return false;
}
if (png_num_palette > palette->ncolors) {
png_num_palette = palette->ncolors;
@@ -462,14 +457,17 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
}
}
}
+
+ return true;
}
SDL_Surface *IMG_LoadPNG_IO(SDL_IOStream *src)
{
Sint64 start;
struct loadpng_vars vars;
+ bool success;
- if ( !src ) {
+ if (!src) {
/* The error message has been set in SDL_IOFromFile */
return NULL;
}
@@ -481,8 +479,7 @@ SDL_Surface *IMG_LoadPNG_IO(SDL_IOStream *src)
start = SDL_TellIO(src);
SDL_zero(vars);
- LIBPNG_LoadPNG_IO(src, &vars);
-
+ success = LIBPNG_LoadPNG_IO(src, &vars);
if (vars.png_ptr) {
lib.png_destroy_read_struct(&vars.png_ptr,
vars.info_ptr ? &vars.info_ptr : (png_infopp)0,
@@ -491,16 +488,20 @@ SDL_Surface *IMG_LoadPNG_IO(SDL_IOStream *src)
if (vars.row_pointers) {
SDL_free(vars.row_pointers);
}
+ if (success) {
+ return vars.surface;
+ }
+
+ /* this may clobber a set error if seek fails: don't care. */
+ SDL_SeekIO(src, start, SDL_IO_SEEK_SET);
+ if (vars.surface) {
+ SDL_DestroySurface(vars.surface);
+ }
if (vars.error) {
- SDL_SeekIO(src, start, SDL_IO_SEEK_SET);
- if (vars.surface) {
- SDL_DestroySurface(vars.surface);
- vars.surface = NULL;
- }
SDL_SetError("%s", vars.error);
}
- return vars.surface;
+ return NULL;
}
#elif defined(USE_STBIMAGE)