Skip to content

Commit 0dfa3d8

Browse files
sezerosezero
committed
Factor out parts of IMG_LoadJPG where locals can be clobbered by longjmp
Also sanitize error setting a bit in IMG_LoadPNG() Fixes #435
1 parent 3205cf4 commit 0dfa3d8

File tree

2 files changed

+101
-79
lines changed

2 files changed

+101
-79
lines changed

‎src/IMG_jpg.c‎

Lines changed: 76 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ int IMG_InitJPG()
124124

125125
return 0;
126126
}
127+
127128
void IMG_QuitJPG()
128129
{
129130
if ( lib.loaded == 0 ) {
@@ -337,89 +338,106 @@ static void output_no_message(j_common_ptr cinfo)
337338
(void)cinfo;
338339
}
339340

340-
/* Load a JPEG type image from an SDL datasource */
341-
SDL_Surface *IMG_LoadJPG_RW(SDL_RWops *src)
342-
{
343-
Sint64 start;
341+
struct loadjpeg_vars {
342+
const char *error;
343+
SDL_Surface *surface;
344344
struct jpeg_decompress_struct cinfo;
345-
JSAMPROW rowptr[1];
346-
SDL_Surface *surface = NULL;
347345
struct my_error_mgr jerr;
346+
};
348347

349-
if ( !src ) {
350-
/* The error message has been set in SDL_RWFromFile */
351-
return NULL;
352-
}
353-
start = SDL_RWtell(src);
354-
355-
if ( (IMG_Init(IMG_INIT_JPG) & IMG_INIT_JPG) == 0 ) {
356-
return NULL;
357-
}
348+
/* Load a JPEG type image from an SDL datasource */
349+
static SDL_bool LIBJPEG_LoadJPG_RW(SDL_RWops *src, struct loadjpeg_vars *vars)
350+
{
351+
JSAMPROW rowptr[1];
358352

359353
/* Create a decompression structure and load the JPEG header */
360-
cinfo.err = lib.jpeg_std_error(&jerr.errmgr);
361-
jerr.errmgr.error_exit = my_error_exit;
362-
jerr.errmgr.output_message = output_no_message;
363-
#ifdef _MSC_VER
364-
#pragma warning(disable:4611) /* warning C4611: interaction between '_setjmp' and C++ object destruction is non-portable */
365-
#endif
366-
if(setjmp(jerr.escape)) {
354+
vars->cinfo.err = lib.jpeg_std_error(&vars->jerr.errmgr);
355+
vars->jerr.errmgr.error_exit = my_error_exit;
356+
vars->jerr.errmgr.output_message = output_no_message;
357+
if (setjmp(vars->jerr.escape)) {
367358
/* If we get here, libjpeg found an error */
368-
lib.jpeg_destroy_decompress(&cinfo);
369-
if ( surface != NULL ) {
370-
SDL_FreeSurface(surface);
371-
}
372-
SDL_RWseek(src, start, RW_SEEK_SET);
373-
IMG_SetError("JPEG loading error");
374-
return NULL;
359+
lib.jpeg_destroy_decompress(&vars->cinfo);
360+
vars->error = "JPEG loading error";
361+
return SDL_FALSE;
375362
}
376363

377-
lib.jpeg_create_decompress(&cinfo);
378-
jpeg_SDL_RW_src(&cinfo, src);
379-
lib.jpeg_read_header(&cinfo, TRUE);
364+
lib.jpeg_create_decompress(&vars->cinfo);
365+
jpeg_SDL_RW_src(&vars->cinfo, src);
366+
lib.jpeg_read_header(&vars->cinfo, TRUE);
380367

381-
if(cinfo.num_components == 4) {
368+
if (vars->cinfo.num_components == 4) {
382369
/* Set 32-bit Raw output */
383-
cinfo.out_color_space = JCS_CMYK;
384-
cinfo.quantize_colors = FALSE;
385-
lib.jpeg_calc_output_dimensions(&cinfo);
370+
vars->cinfo.out_color_space = JCS_CMYK;
371+
vars->cinfo.quantize_colors = FALSE;
372+
lib.jpeg_calc_output_dimensions(&vars->cinfo);
386373

387374
/* Allocate an output surface to hold the image */
388-
surface = SDL_CreateRGBSurfaceWithFormat(0, cinfo.output_width, cinfo.output_height, 0, SDL_PIXELFORMAT_BGRA32);
375+
vars->surface = SDL_CreateRGBSurfaceWithFormat(0, vars->cinfo.output_width, vars->cinfo.output_height, 0, SDL_PIXELFORMAT_BGRA32);
389376
} else {
390377
/* Set 24-bit RGB output */
391-
cinfo.out_color_space = JCS_RGB;
392-
cinfo.quantize_colors = FALSE;
378+
vars->cinfo.out_color_space = JCS_RGB;
379+
vars->cinfo.quantize_colors = FALSE;
393380
#ifdef FAST_JPEG
394-
cinfo.scale_num = 1;
395-
cinfo.scale_denom = 1;
396-
cinfo.dct_method = JDCT_FASTEST;
397-
cinfo.do_fancy_upsampling = FALSE;
381+
vars->cinfo.scale_num = 1;
382+
vars->cinfo.scale_denom = 1;
383+
vars->cinfo.dct_method = JDCT_FASTEST;
384+
vars->cinfo.do_fancy_upsampling = FALSE;
398385
#endif
399-
lib.jpeg_calc_output_dimensions(&cinfo);
386+
lib.jpeg_calc_output_dimensions(&vars->cinfo);
400387

401388
/* Allocate an output surface to hold the image */
402-
surface = SDL_CreateRGBSurfaceWithFormat(0, cinfo.output_width, cinfo.output_height, 0, SDL_PIXELFORMAT_RGB24);
389+
vars->surface = SDL_CreateRGBSurfaceWithFormat(0, vars->cinfo.output_width, vars->cinfo.output_height, 0, SDL_PIXELFORMAT_RGB24);
403390
}
404391

405-
if ( surface == NULL ) {
406-
lib.jpeg_destroy_decompress(&cinfo);
407-
SDL_RWseek(src, start, RW_SEEK_SET);
408-
IMG_SetError("Out of memory");
409-
return NULL;
392+
if (!vars->surface) {
393+
lib.jpeg_destroy_decompress(&vars->cinfo);
394+
return SDL_FALSE;
410395
}
411396

412397
/* Decompress the image */
413-
lib.jpeg_start_decompress(&cinfo);
414-
while ( cinfo.output_scanline < cinfo.output_height ) {
415-
rowptr[0] = (JSAMPROW)(Uint8 *)surface->pixels +
416-
cinfo.output_scanline * surface->pitch;
417-
lib.jpeg_read_scanlines(&cinfo, rowptr, (JDIMENSION) 1);
398+
lib.jpeg_start_decompress(&vars->cinfo);
399+
while (vars->cinfo.output_scanline < vars->cinfo.output_height) {
400+
rowptr[0] = (JSAMPROW)(Uint8 *)vars->surface->pixels +
401+
vars->cinfo.output_scanline * vars->surface->pitch;
402+
lib.jpeg_read_scanlines(&vars->cinfo, rowptr, (JDIMENSION) 1);
403+
}
404+
lib.jpeg_finish_decompress(&vars->cinfo);
405+
lib.jpeg_destroy_decompress(&vars->cinfo);
406+
407+
return SDL_TRUE;
408+
}
409+
410+
SDL_Surface *IMG_LoadJPG_RW(SDL_RWops *src)
411+
{
412+
Sint64 start;
413+
struct loadjpeg_vars vars;
414+
415+
if (!src) {
416+
/* The error message has been set in SDL_RWFromFile */
417+
return NULL;
418+
}
419+
420+
if ((IMG_Init(IMG_INIT_JPG) & IMG_INIT_JPG) == 0) {
421+
return NULL;
422+
}
423+
424+
start = SDL_RWtell(src);
425+
SDL_zero(vars);
426+
427+
if (LIBJPEG_LoadJPG_RW(src, &vars)) {
428+
return vars.surface;
429+
}
430+
431+
/* this may clobber a set error if seek fails: don't care. */
432+
SDL_RWseek(src, start, RW_SEEK_SET);
433+
if (vars.surface) {
434+
SDL_FreeSurface(vars.surface);
435+
}
436+
if (vars.error) {
437+
IMG_SetError("%s", vars.error);
418438
}
419-
lib.jpeg_finish_decompress(&cinfo);
420-
lib.jpeg_destroy_decompress(&cinfo);
421439

422-
return(surface);
440+
return NULL;
423441
}
424442

425443
#define OUTPUT_BUFFER_SIZE 4096

‎src/IMG_png.c‎

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ int IMG_InitPNG()
186186

187187
return 0;
188188
}
189+
189190
void IMG_QuitPNG()
190191
{
191192
if ( lib.loaded == 0 ) {
@@ -241,7 +242,7 @@ struct loadpng_vars {
241242
png_bytep *row_pointers;
242243
};
243244

244-
static void LIBPNG_LoadPNG_RW(SDL_RWops *src, struct loadpng_vars *vars)
245+
static SDL_bool LIBPNG_LoadPNG_RW(SDL_RWops *src, struct loadpng_vars *vars)
245246
{
246247
png_uint_32 width, height;
247248
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)
256257
NULL,NULL,NULL);
257258
if (vars->png_ptr == NULL) {
258259
vars->error = "Couldn't allocate memory for PNG file or incompatible PNG dll";
259-
return;
260+
return SDL_FALSE;
260261
}
261262

262263
/* Allocate/initialize the memory for image information. REQUIRED. */
263264
vars->info_ptr = lib.png_create_info_struct(vars->png_ptr);
264265
if (vars->info_ptr == NULL) {
265266
vars->error = "Couldn't create image information for PNG file";
266-
return;
267+
return SDL_FALSE;
267268
}
268269

269270
/* Set error handling if you are using setjmp/longjmp method (this is
@@ -272,17 +273,14 @@ static void LIBPNG_LoadPNG_RW(SDL_RWops *src, struct loadpng_vars *vars)
272273
*/
273274

274275
#ifdef PNG_SETJMP_SUPPORTED
275-
#ifdef _MSC_VER
276-
#pragma warning(disable:4611) /* warning C4611: interaction between '_setjmp' and C++ object destruction is non-portable */
277-
#endif
278276
#ifndef LIBPNG_VERSION_12
279277
if (setjmp(*lib.png_set_longjmp_fn(vars->png_ptr, longjmp, sizeof(jmp_buf))))
280278
#else
281279
if (setjmp(vars->png_ptr->jmpbuf))
282280
#endif
283281
{
284282
vars->error = "Error reading the PNG file.";
285-
return;
283+
return SDL_FALSE;
286284
}
287285
#endif
288286
/* Set up the input control */
@@ -389,8 +387,7 @@ static void LIBPNG_LoadPNG_RW(SDL_RWops *src, struct loadpng_vars *vars)
389387

390388
vars->surface = SDL_CreateRGBSurfaceWithFormat(0, width, height, 0, format);
391389
if (vars->surface == NULL) {
392-
vars->error = SDL_GetError();
393-
return;
390+
return SDL_FALSE;
394391
}
395392

396393
if (ckey != -1) {
@@ -408,7 +405,7 @@ static void LIBPNG_LoadPNG_RW(SDL_RWops *src, struct loadpng_vars *vars)
408405
vars->row_pointers = (png_bytep*) SDL_malloc(sizeof(png_bytep)*height);
409406
if (!vars->row_pointers) {
410407
vars->error = "Out of memory";
411-
return;
408+
return SDL_FALSE;
412409
}
413410
for (row = 0; row < (int)height; row++) {
414411
vars->row_pointers[row] = (png_bytep)
@@ -448,27 +445,29 @@ static void LIBPNG_LoadPNG_RW(SDL_RWops *src, struct loadpng_vars *vars)
448445
}
449446
}
450447
}
448+
449+
return SDL_TRUE;
451450
}
452451

453452
SDL_Surface *IMG_LoadPNG_RW(SDL_RWops *src)
454453
{
455454
Sint64 start;
456455
struct loadpng_vars vars;
456+
SDL_bool success;
457457

458-
if ( !src ) {
458+
if (!src) {
459459
/* The error message has been set in SDL_RWFromFile */
460460
return NULL;
461461
}
462462

463-
if ( (IMG_Init(IMG_INIT_PNG) & IMG_INIT_PNG) == 0 ) {
463+
if ((IMG_Init(IMG_INIT_PNG) & IMG_INIT_PNG) == 0) {
464464
return NULL;
465465
}
466466

467467
start = SDL_RWtell(src);
468468
SDL_zero(vars);
469469

470-
LIBPNG_LoadPNG_RW(src, &vars);
471-
470+
success = LIBPNG_LoadPNG_RW(src, &vars);
472471
if (vars.png_ptr) {
473472
lib.png_destroy_read_struct(&vars.png_ptr,
474473
vars.info_ptr ? &vars.info_ptr : (png_infopp)0,
@@ -477,16 +476,20 @@ SDL_Surface *IMG_LoadPNG_RW(SDL_RWops *src)
477476
if (vars.row_pointers) {
478477
SDL_free(vars.row_pointers);
479478
}
479+
if (success) {
480+
return vars.surface;
481+
}
482+
483+
/* this may clobber a set error if seek fails: don't care. */
484+
SDL_RWseek(src, start, RW_SEEK_SET);
485+
if (vars.surface) {
486+
SDL_FreeSurface(vars.surface);
487+
}
480488
if (vars.error) {
481-
SDL_RWseek(src, start, RW_SEEK_SET);
482-
if (vars.surface) {
483-
SDL_FreeSurface(vars.surface);
484-
vars.surface = NULL;
485-
}
486489
IMG_SetError("%s", vars.error);
487490
}
488491

489-
return vars.surface;
492+
return NULL;
490493
}
491494

492495
#elif defined(USE_STBIMAGE)
@@ -603,7 +606,8 @@ static int LIBPNG_SavePNG_RW(struct savepng_vars *vars, SDL_Surface *surface, SD
603606

604607
vars->png_ptr = lib.png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
605608
if (vars->png_ptr == NULL) {
606-
return IMG_SetError("Couldn't allocate memory for PNG file or incompatible PNG dll");
609+
vars->error = "Couldn't allocate memory for PNG file or incompatible PNG dll";
610+
return -1;
607611
}
608612

609613
vars->info_ptr = lib.png_create_info_struct(vars->png_ptr);

0 commit comments

Comments
 (0)