From 0dfa3d848c577481c4f97058267eda17b7f61aa8 Mon Sep 17 00:00:00 2001
From: Ozkan Sezer <[EMAIL REDACTED]>
Date: Wed, 9 Apr 2025 07:23:02 +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 | 46 +++++++++--------
2 files changed, 101 insertions(+), 79 deletions(-)
diff --git a/src/IMG_jpg.c b/src/IMG_jpg.c
index 98050b9d7..4b6b8f587 100644
--- a/src/IMG_jpg.c
+++ b/src/IMG_jpg.c
@@ -124,6 +124,7 @@ int IMG_InitJPG()
return 0;
}
+
void IMG_QuitJPG()
{
if ( lib.loaded == 0 ) {
@@ -337,89 +338,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_RW(SDL_RWops *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_RWFromFile */
- return NULL;
- }
- start = SDL_RWtell(src);
-
- if ( (IMG_Init(IMG_INIT_JPG) & IMG_INIT_JPG) == 0 ) {
- return NULL;
- }
+/* Load a JPEG type image from an SDL datasource */
+static SDL_bool LIBJPEG_LoadJPG_RW(SDL_RWops *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_FreeSurface(surface);
- }
- SDL_RWseek(src, start, RW_SEEK_SET);
- IMG_SetError("JPEG loading error");
- return NULL;
+ lib.jpeg_destroy_decompress(&vars->cinfo);
+ vars->error = "JPEG loading error";
+ return SDL_FALSE;
}
- lib.jpeg_create_decompress(&cinfo);
- jpeg_SDL_RW_src(&cinfo, src);
- lib.jpeg_read_header(&cinfo, TRUE);
+ lib.jpeg_create_decompress(&vars->cinfo);
+ jpeg_SDL_RW_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_CreateRGBSurfaceWithFormat(0, cinfo.output_width, cinfo.output_height, 0, SDL_PIXELFORMAT_BGRA32);
+ vars->surface = SDL_CreateRGBSurfaceWithFormat(0, vars->cinfo.output_width, vars->cinfo.output_height, 0, 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_CreateRGBSurfaceWithFormat(0, cinfo.output_width, cinfo.output_height, 0, SDL_PIXELFORMAT_RGB24);
+ vars->surface = SDL_CreateRGBSurfaceWithFormat(0, vars->cinfo.output_width, vars->cinfo.output_height, 0, SDL_PIXELFORMAT_RGB24);
}
- if ( surface == NULL ) {
- lib.jpeg_destroy_decompress(&cinfo);
- SDL_RWseek(src, start, RW_SEEK_SET);
- IMG_SetError("Out of memory");
- return NULL;
+ if (!vars->surface) {
+ lib.jpeg_destroy_decompress(&vars->cinfo);
+ return SDL_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 SDL_TRUE;
+}
+
+SDL_Surface *IMG_LoadJPG_RW(SDL_RWops *src)
+{
+ Sint64 start;
+ struct loadjpeg_vars vars;
+
+ if (!src) {
+ /* The error message has been set in SDL_RWFromFile */
+ return NULL;
+ }
+
+ if ((IMG_Init(IMG_INIT_JPG) & IMG_INIT_JPG) == 0) {
+ return NULL;
+ }
+
+ start = SDL_RWtell(src);
+ SDL_zero(vars);
+
+ if (LIBJPEG_LoadJPG_RW(src, &vars)) {
+ return vars.surface;
+ }
+
+ /* this may clobber a set error if seek fails: don't care. */
+ SDL_RWseek(src, start, RW_SEEK_SET);
+ if (vars.surface) {
+ SDL_FreeSurface(vars.surface);
+ }
+ if (vars.error) {
+ IMG_SetError("%s", vars.error);
}
- lib.jpeg_finish_decompress(&cinfo);
- lib.jpeg_destroy_decompress(&cinfo);
- return(surface);
+ return NULL;
}
#define OUTPUT_BUFFER_SIZE 4096
diff --git a/src/IMG_png.c b/src/IMG_png.c
index cdee02cfb..5f5c74369 100644
--- a/src/IMG_png.c
+++ b/src/IMG_png.c
@@ -186,6 +186,7 @@ int IMG_InitPNG()
return 0;
}
+
void IMG_QuitPNG()
{
if ( lib.loaded == 0 ) {
@@ -241,7 +242,7 @@ struct loadpng_vars {
png_bytep *row_pointers;
};
-static void LIBPNG_LoadPNG_RW(SDL_RWops *src, struct loadpng_vars *vars)
+static SDL_bool LIBPNG_LoadPNG_RW(SDL_RWops *src, struct loadpng_vars *vars)
{
png_uint_32 width, height;
int bit_depth, color_type, interlace_type, num_channels;
@@ -256,14 +257,14 @@ static void LIBPNG_LoadPNG_RW(SDL_RWops *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 SDL_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 SDL_FALSE;
}
/* Set error handling if you are using setjmp/longjmp method (this is
@@ -272,9 +273,6 @@ static void LIBPNG_LoadPNG_RW(SDL_RWops *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
@@ -282,7 +280,7 @@ static void LIBPNG_LoadPNG_RW(SDL_RWops *src, struct loadpng_vars *vars)
#endif
{
vars->error = "Error reading the PNG file.";
- return;
+ return SDL_FALSE;
}
#endif
/* Set up the input control */
@@ -389,8 +387,7 @@ static void LIBPNG_LoadPNG_RW(SDL_RWops *src, struct loadpng_vars *vars)
vars->surface = SDL_CreateRGBSurfaceWithFormat(0, width, height, 0, format);
if (vars->surface == NULL) {
- vars->error = SDL_GetError();
- return;
+ return SDL_FALSE;
}
if (ckey != -1) {
@@ -408,7 +405,7 @@ static void LIBPNG_LoadPNG_RW(SDL_RWops *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 SDL_FALSE;
}
for (row = 0; row < (int)height; row++) {
vars->row_pointers[row] = (png_bytep)
@@ -448,27 +445,29 @@ static void LIBPNG_LoadPNG_RW(SDL_RWops *src, struct loadpng_vars *vars)
}
}
}
+
+ return SDL_TRUE;
}
SDL_Surface *IMG_LoadPNG_RW(SDL_RWops *src)
{
Sint64 start;
struct loadpng_vars vars;
+ SDL_bool success;
- if ( !src ) {
+ if (!src) {
/* The error message has been set in SDL_RWFromFile */
return NULL;
}
- if ( (IMG_Init(IMG_INIT_PNG) & IMG_INIT_PNG) == 0 ) {
+ if ((IMG_Init(IMG_INIT_PNG) & IMG_INIT_PNG) == 0) {
return NULL;
}
start = SDL_RWtell(src);
SDL_zero(vars);
- LIBPNG_LoadPNG_RW(src, &vars);
-
+ success = LIBPNG_LoadPNG_RW(src, &vars);
if (vars.png_ptr) {
lib.png_destroy_read_struct(&vars.png_ptr,
vars.info_ptr ? &vars.info_ptr : (png_infopp)0,
@@ -477,16 +476,20 @@ SDL_Surface *IMG_LoadPNG_RW(SDL_RWops *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_RWseek(src, start, RW_SEEK_SET);
+ if (vars.surface) {
+ SDL_FreeSurface(vars.surface);
+ }
if (vars.error) {
- SDL_RWseek(src, start, RW_SEEK_SET);
- if (vars.surface) {
- SDL_FreeSurface(vars.surface);
- vars.surface = NULL;
- }
IMG_SetError("%s", vars.error);
}
- return vars.surface;
+ return NULL;
}
#elif defined(USE_STBIMAGE)
@@ -603,7 +606,8 @@ static int LIBPNG_SavePNG_RW(struct savepng_vars *vars, SDL_Surface *surface, SD
vars->png_ptr = lib.png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
if (vars->png_ptr == NULL) {
- return IMG_SetError("Couldn't allocate memory for PNG file or incompatible PNG dll");
+ vars->error = "Couldn't allocate memory for PNG file or incompatible PNG dll";
+ return -1;
}
vars->info_ptr = lib.png_create_info_struct(vars->png_ptr);