bound gif palette index reads to the color table size#7541
Conversation
|
Added a test under renderers (gif_palette_bounds.map). It runs two crafted GIFs through readGIF via mapserv: one with a background index past the 2-entry palette, and one with no color map at all. Both come back as the deterministic error page, so they compare cleanly, and under the --run_under_asan job the unpatched decoder trips the heap-buffer-overflow on the background fill before the message ever gets produced. Put the .gif fixtures in misc/data and the expected output in renderers/expected. |
|
Thanks @nvxbug, your new tests are failing, please make sure that the tests pass. |
The no-colormap and img-size-exceeds-screen error paths returned without releasing the pixel buffer or the giflib handle, so the regression GIFs tripped LeakSanitizer under the ASAN test job. Release both before returning.
|
Tracked it down: the two error paths those fixtures hit (no color map, and img size exceeds screen) were returning without freeing the pixel buffer or closing the giflib handle, so under the ASAN job LeakSanitizer flagged a leak and the process bailed before the error page got written out, which is why the results looked like a mismatch. Pushed a fix that releases both before returning on those paths. Both GIFs now come back with their error page cleanly under ASAN. |
rouault
added
Security
backport branch-8-6
3 participants
readGIF in mapimageio.c indexes the global color table with values taken straight from the file: image->SBackGroundColor for the initial background fill, and the decoded pixel value for every image pixel. giflib does not check either against the table size, so a GIF whose background index or pixel values are larger than the palette walks past cmap->Colors and reads out of bounds. A 2-color global table with a background index of 255 reproduces it, with ASAN reporting a heap read 759 bytes past the color table.
Clamp the background index and skip palette lookups for pixel values that fall outside cmap->ColorCount, rendering those as transparent like the existing transparent-index branch. The check sits in the decoder because the indices come from the file and nothing upstream knows the table size.