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.
Fix to buffer issue where stb_strscpy would use + 1 for buffer length when stb_p_strcpy_s is called, causing a stack variable corrupted issue.
Fix to readdir_raw to no longer account for stb_strscpy having + 1 in buffer length.
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").
I propose to remove this line because f->valid_bits will never be less than zero since, in the while loop, you're adding 8 to it. Therefore, it will always evaluate to false. This is to help remove redundant code.
The documentation of that operation already said:
> Returns a pointer to the first uninitialized item added.
This also makes a lot of sense, allowing easy initialization. But the
implementation returned the index of the first uninitialized element
instead.
Randy
Colton G. Rushton
Recep Aslantas
Pedro J. Estébanez
Ryan C. Gordon
Clownacy
Adam Korczynski
Ignacio Castano
Kyle Langley
Andreas Molzer
Zack Middleton
wph612
David Reid
Luca Sas
hashitaku
Sean Barrett
Marco Lizza