The BMP loader already had this hardcoded to (1 << 24) pixels, so this seems
like a good default to apply to all formats, but many apps will want to clamp
this much much lower.
It's possible to craft malicious but valid images that are enormous, causing
stb_image to allocate tons of memory and eat a ton of CPU, so locking these
to a maximum permitted size can save a lot of headaches in the wild.
There are several places where stb_image protects itself from bad data with
STBI_ASSERT macros, but if these are compiled out in release builds the code
will overflow buffers, etc, without warning. If they are left enabled, the
process will crash from assertion failures.
This patch attempts to leave the assertions in place that are meant to verify
the correctness of the interfaces (if the calling function was meant to pass
only 8 or 16 for bit depth, it's reasonable to assert that is accurate), but
changes asserts that are triggered by corrupt or malicious image file data.
Failed asserts were the majority of crashes during fuzzing; now all of these
cases safely report an error back to the calling app.
This causes problems when stb_image tries to do this with stdio callbacks with
a maliciously crafted file (or just an unfortunately corrupt one)...
// calls fread(), sets EOF flag, sets s->read_from_callbacks = 0
stbi__refill_buffer(s);
// calls fseek(), which resets the stream's EOF flag
stbi__skip(some value we just read)
// calls feof(), which always returns false because EOF flag was reset.
while (!stbi__at_eof(s)) {
// never calls fread() because s->read_from_callbacks==0
stbi__refill_buffer(s);
// loop forever
}
To work around this, after seeking, we call fgetc(), which will set the EOF
flag as appropriate, and if not at EOF, we ungetc the byte so future reads
are correct. This fixes the infinite loop.
If realloc fails it returns NULL and out pointer becomes invalid. To
fix this it is necessary to store realloc return value in temporary
pointer and then compare it with NULL. If it equals NULL then return
error and source pointer will still valid.
This error was caught by cppcheck:
Common realloc mistake: 'out' nulled but not freed upon failure.
I must've missed it when I did this for the other image loaders.
Either way, combined with the previous checkin, this should fix
issue #614 properly.
Fixes issue #614.
1. Check not just g->out allocation for failure.
2. If an image descriptor specified a 0-width image, this could be
used to produce an out-of-bounds write.
3. Fix memory leak in case an error occurs during decoding.
Fixes issue #656.
Ryan C. Gordon
Sean Barrett
Brad Weinberger
Try
BlackMATov
Georgy Macharadze
Łukasz Ptak
Fabian Giesen