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.
This fixes two issues with an assert failing. I tested that the
first part fixes#909 and the second fixes#897.
1. Loading 16/24/32-bit BMP from memory caused an assert to fail
(excluding 16-bit BMP with hsz 12).
img_buffer offset was always compared with the buffer for
stbi_load_from_file() but stbi_load_from_memory() uses an external
buffer.
Resolution: Change s->buffer_start to s->img_buffer_original.
2. Loading BMP with large header from file caused assert to fail.
img_buffer points to stbi_uc buffer_start[128] but the largest BMP
supported has a 138 byte header (hsz 124) causing img_buffer to wrap
around to an offset of 10. The assert fails because 138 (header size)
!= 10 (offset in temp read buffer).
Resolution: Add the previously read bytes to the offset in temp read
buffer to get the absolute offset.
The issues were introduced by the commit c440a53d06
("stb_image: fix reading BMP with explicit masks").
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.
Fabian Giesen
Sean Barrett
Colton G. Rushton
Recep Aslantas
Ryan C. Gordon
Randy
Zack Middleton
Luca Sas
Sean Barrett
Brad Weinberger
Try
BlackMATov
Georgy Macharadze
Łukasz Ptak