Skip to content

Commit b2eed83

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 884acf8 commit b2eed83

File tree

2 files changed

+101
-82
lines changed

2 files changed

+101
-82
lines changed

‎src/IMG_jpg.c

+76-58
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ static bool IMG_InitJPG(void)
124124

125125
return true;
126126
}
127+
127128
#if 0
128129
void IMG_QuitJPG(void)
129130
{
@@ -343,89 +344,106 @@ static void output_no_message(j_common_ptr cinfo)
343344
(void)cinfo;
344345
}
345346

346-
/* Load a JPEG type image from an SDL datasource */
347-
SDL_Surface *IMG_LoadJPG_IO(SDL_IOStream *src)
348-
{
349-
Sint64 start;
347+
struct loadjpeg_vars {
348+
const char *error;
349+
SDL_Surface *surface;
350350
struct jpeg_decompress_struct cinfo;
351-
JSAMPROW rowptr[1];
352-
SDL_Surface *surface = NULL;
353351
struct my_error_mgr jerr;
352+
};
354353

355-
if ( !src ) {
356-
/* The error message has been set in SDL_IOFromFile */
357-
return NULL;
358-
}
359-
start = SDL_TellIO(src);
360-
361-
if (!IMG_InitJPG()) {
362-
return NULL;
363-
}
354+
/* Load a JPEG type image from an SDL datasource */
355+
static bool LIBJPEG_LoadJPG_IO(SDL_IOStream *src, struct loadjpeg_vars *vars)
356+
{
357+
JSAMPROW rowptr[1];
364358

365359
/* Create a decompression structure and load the JPEG header */
366-
cinfo.err = lib.jpeg_std_error(&jerr.errmgr);
367-
jerr.errmgr.error_exit = my_error_exit;
368-
jerr.errmgr.output_message = output_no_message;
369-
#ifdef _MSC_VER
370-
#pragma warning(disable:4611) /* warning C4611: interaction between '_setjmp' and C++ object destruction is non-portable */
371-
#endif
372-
if (setjmp(jerr.escape)) {
360+
vars->cinfo.err = lib.jpeg_std_error(&vars->jerr.errmgr);
361+
vars->jerr.errmgr.error_exit = my_error_exit;
362+
vars->jerr.errmgr.output_message = output_no_message;
363+
if (setjmp(vars->jerr.escape)) {
373364
/* If we get here, libjpeg found an error */
374-
lib.jpeg_destroy_decompress(&cinfo);
375-
if ( surface != NULL ) {
376-
SDL_DestroySurface(surface);
377-
}
378-
SDL_SeekIO(src, start, SDL_IO_SEEK_SET);
379-
SDL_SetError("JPEG loading error");
380-
return NULL;
365+
lib.jpeg_destroy_decompress(&vars->cinfo);
366+
vars->error = "JPEG loading error";
367+
return false;
381368
}
382369

383-
lib.jpeg_create_decompress(&cinfo);
384-
jpeg_SDL_IO_src(&cinfo, src);
385-
lib.jpeg_read_header(&cinfo, TRUE);
370+
lib.jpeg_create_decompress(&vars->cinfo);
371+
jpeg_SDL_IO_src(&vars->cinfo, src);
372+
lib.jpeg_read_header(&vars->cinfo, TRUE);
386373

387-
if (cinfo.num_components == 4) {
374+
if (vars->cinfo.num_components == 4) {
388375
/* Set 32-bit Raw output */
389-
cinfo.out_color_space = JCS_CMYK;
390-
cinfo.quantize_colors = FALSE;
391-
lib.jpeg_calc_output_dimensions(&cinfo);
376+
vars->cinfo.out_color_space = JCS_CMYK;
377+
vars->cinfo.quantize_colors = FALSE;
378+
lib.jpeg_calc_output_dimensions(&vars->cinfo);
392379

393380
/* Allocate an output surface to hold the image */
394-
surface = SDL_CreateSurface(cinfo.output_width, cinfo.output_height, SDL_PIXELFORMAT_BGRA32);
381+
vars->surface = SDL_CreateSurface(vars->cinfo.output_width, vars->cinfo.output_height, SDL_PIXELFORMAT_BGRA32);
395382
} else {
396383
/* Set 24-bit RGB output */
397-
cinfo.out_color_space = JCS_RGB;
398-
cinfo.quantize_colors = FALSE;
384+
vars->cinfo.out_color_space = JCS_RGB;
385+
vars->cinfo.quantize_colors = FALSE;
399386
#ifdef FAST_JPEG
400-
cinfo.scale_num = 1;
401-
cinfo.scale_denom = 1;
402-
cinfo.dct_method = JDCT_FASTEST;
403-
cinfo.do_fancy_upsampling = FALSE;
387+
vars->cinfo.scale_num = 1;
388+
vars->cinfo.scale_denom = 1;
389+
vars->cinfo.dct_method = JDCT_FASTEST;
390+
vars->cinfo.do_fancy_upsampling = FALSE;
404391
#endif
405-
lib.jpeg_calc_output_dimensions(&cinfo);
392+
lib.jpeg_calc_output_dimensions(&vars->cinfo);
406393

407394
/* Allocate an output surface to hold the image */
408-
surface = SDL_CreateSurface(cinfo.output_width, cinfo.output_height, SDL_PIXELFORMAT_RGB24);
395+
vars->surface = SDL_CreateSurface(vars->cinfo.output_width, vars->cinfo.output_height, SDL_PIXELFORMAT_RGB24);
409396
}
410397

411-
if ( surface == NULL ) {
412-
lib.jpeg_destroy_decompress(&cinfo);
413-
SDL_SeekIO(src, start, SDL_IO_SEEK_SET);
414-
SDL_SetError("Out of memory");
415-
return NULL;
398+
if (!vars->surface) {
399+
lib.jpeg_destroy_decompress(&vars->cinfo);
400+
return false;
416401
}
417402

418403
/* Decompress the image */
419-
lib.jpeg_start_decompress(&cinfo);
420-
while ( cinfo.output_scanline < cinfo.output_height ) {
421-
rowptr[0] = (JSAMPROW)(Uint8 *)surface->pixels +
422-
cinfo.output_scanline * surface->pitch;
423-
lib.jpeg_read_scanlines(&cinfo, rowptr, (JDIMENSION) 1);
404+
lib.jpeg_start_decompress(&vars->cinfo);
405+
while (vars->cinfo.output_scanline < vars->cinfo.output_height) {
406+
rowptr[0] = (JSAMPROW)(Uint8 *)vars->surface->pixels +
407+
vars->cinfo.output_scanline * vars->surface->pitch;
408+
lib.jpeg_read_scanlines(&vars->cinfo, rowptr, (JDIMENSION) 1);
409+
}
410+
lib.jpeg_finish_decompress(&vars->cinfo);
411+
lib.jpeg_destroy_decompress(&vars->cinfo);
412+
413+
return true;
414+
}
415+
416+
SDL_Surface *IMG_LoadJPG_IO(SDL_IOStream *src)
417+
{
418+
Sint64 start;
419+
struct loadjpeg_vars vars;
420+
421+
if (!src) {
422+
/* The error message has been set in SDL_IOFromFile */
423+
return NULL;
424424
}
425-
lib.jpeg_finish_decompress(&cinfo);
426-
lib.jpeg_destroy_decompress(&cinfo);
427425

428-
return surface;
426+
if (!IMG_InitJPG()) {
427+
return NULL;
428+
}
429+
430+
start = SDL_TellIO(src);
431+
SDL_zero(vars);
432+
433+
if (LIBJPEG_LoadJPG_IO(src, &vars)) {
434+
return vars.surface;
435+
}
436+
437+
/* this may clobber a set error if seek fails: don't care. */
438+
SDL_SeekIO(src, start, SDL_IO_SEEK_SET);
439+
if (vars.surface) {
440+
SDL_DestroySurface(vars.surface);
441+
}
442+
if (vars.error) {
443+
SDL_SetError("%s", vars.error);
444+
}
445+
446+
return NULL;
429447
}
430448

431449
#define OUTPUT_BUFFER_SIZE 4096

‎src/IMG_png.c

+25-24
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ static bool IMG_InitPNG(void)
186186

187187
return true;
188188
}
189+
189190
#if 0
190191
void IMG_QuitPNG(void)
191192
{
@@ -243,7 +244,7 @@ struct loadpng_vars {
243244
png_bytep *row_pointers;
244245
};
245246

246-
static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
247+
static bool LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
247248
{
248249
png_uint_32 width, height;
249250
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)
257258
NULL,NULL,NULL);
258259
if (vars->png_ptr == NULL) {
259260
vars->error = "Couldn't allocate memory for PNG file or incompatible PNG dll";
260-
return;
261+
return false;
261262
}
262263

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

270271
/* Set error handling if you are using setjmp/longjmp method (this is
@@ -273,17 +274,14 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
273274
*/
274275

275276
#ifdef PNG_SETJMP_SUPPORTED
276-
#ifdef _MSC_VER
277-
#pragma warning(disable:4611) /* warning C4611: interaction between '_setjmp' and C++ object destruction is non-portable */
278-
#endif
279277
#ifndef LIBPNG_VERSION_12
280278
if (setjmp(*lib.png_set_longjmp_fn(vars->png_ptr, longjmp, sizeof(jmp_buf))))
281279
#else
282280
if (setjmp(vars->png_ptr->jmpbuf))
283281
#endif
284282
{
285283
vars->error = "Error reading the PNG file.";
286-
return;
284+
return false;
287285
}
288286
#endif
289287
/* Set up the input control */
@@ -390,8 +388,7 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
390388

391389
vars->surface = SDL_CreateSurface(width, height, format);
392390
if (vars->surface == NULL) {
393-
vars->error = SDL_GetError();
394-
return;
391+
return false;
395392
}
396393

397394
if (ckey != -1) {
@@ -409,7 +406,7 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
409406
vars->row_pointers = (png_bytep*) SDL_malloc(sizeof(png_bytep)*height);
410407
if (!vars->row_pointers) {
411408
vars->error = "Out of memory";
412-
return;
409+
return false;
413410
}
414411
for (row = 0; row < (int)height; row++) {
415412
vars->row_pointers[row] = (png_bytep)
@@ -429,15 +426,14 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
429426

430427
/* Load the palette, if any */
431428
if (SDL_ISPIXELFORMAT_INDEXED(vars->surface->format)) {
432-
SDL_Palette *palette = NULL;
429+
SDL_Palette *palette;
433430
int png_num_palette;
434431
png_colorp png_palette;
435432
lib.png_get_PLTE(vars->png_ptr, vars->info_ptr, &png_palette, &png_num_palette);
436433
if (color_type == PNG_COLOR_TYPE_GRAY) {
437434
palette = SDL_CreateSurfacePalette(vars->surface);
438435
if (!palette) {
439-
vars->error = SDL_GetError();
440-
return;
436+
return false;
441437
}
442438
for (i = 0; i < 256; i++) {
443439
palette->colors[i].r = (Uint8)i;
@@ -447,8 +443,7 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
447443
} else if (png_num_palette > 0 ) {
448444
palette = SDL_CreateSurfacePalette(vars->surface);
449445
if (!palette) {
450-
vars->error = SDL_GetError();
451-
return;
446+
return false;
452447
}
453448
if (png_num_palette > palette->ncolors) {
454449
png_num_palette = palette->ncolors;
@@ -462,14 +457,17 @@ static void LIBPNG_LoadPNG_IO(SDL_IOStream *src, struct loadpng_vars *vars)
462457
}
463458
}
464459
}
460+
461+
return true;
465462
}
466463

467464
SDL_Surface *IMG_LoadPNG_IO(SDL_IOStream *src)
468465
{
469466
Sint64 start;
470467
struct loadpng_vars vars;
468+
bool success;
471469

472-
if ( !src ) {
470+
if (!src) {
473471
/* The error message has been set in SDL_IOFromFile */
474472
return NULL;
475473
}
@@ -481,8 +479,7 @@ SDL_Surface *IMG_LoadPNG_IO(SDL_IOStream *src)
481479
start = SDL_TellIO(src);
482480
SDL_zero(vars);
483481

484-
LIBPNG_LoadPNG_IO(src, &vars);
485-
482+
success = LIBPNG_LoadPNG_IO(src, &vars);
486483
if (vars.png_ptr) {
487484
lib.png_destroy_read_struct(&vars.png_ptr,
488485
vars.info_ptr ? &vars.info_ptr : (png_infopp)0,
@@ -491,16 +488,20 @@ SDL_Surface *IMG_LoadPNG_IO(SDL_IOStream *src)
491488
if (vars.row_pointers) {
492489
SDL_free(vars.row_pointers);
493490
}
491+
if (success) {
492+
return vars.surface;
493+
}
494+
495+
/* this may clobber a set error if seek fails: don't care. */
496+
SDL_SeekIO(src, start, SDL_IO_SEEK_SET);
497+
if (vars.surface) {
498+
SDL_DestroySurface(vars.surface);
499+
}
494500
if (vars.error) {
495-
SDL_SeekIO(src, start, SDL_IO_SEEK_SET);
496-
if (vars.surface) {
497-
SDL_DestroySurface(vars.surface);
498-
vars.surface = NULL;
499-
}
500501
SDL_SetError("%s", vars.error);
501502
}
502503

503-
return vars.surface;
504+
return NULL;
504505
}
505506

506507
#elif defined(USE_STBIMAGE)

0 commit comments

Comments
 (0)