Commit fed846508577419ec0bacb6d1bbbb9074c83c65f

Ryan C. Gordon 2021-05-27T10:40:41

loadbmp: Attempt to handle small palettes better. Only adjust the biClrUsed field if it is set to zero in the bitmap, and make some effort to make sure we don't overflow a buffer in any case. This was triggering an issue with the sailboat bmp used for testpalette.c in SDL 1.2, which is an 8-bit paletted image with 66 palette entries instead of 256. See discussion at https://github.com/libsdl-org/sdl12-compat/issues/63 This change might be a problem, but there's no indication this code, which originally landed in SDL_image 17 years ago with a large rewrite, is actually fixing a specific issue. I'm also not sure we should actually make an effort to accept a bmp that has a biClrUsed field that is both non-zero and _also_ incorrect.

diff --git a/src/video/SDL_bmp.c b/src/video/SDL_bmp.c
index 67445c7..ecf31af 100644
--- a/src/video/SDL_bmp.c
+++ b/src/video/SDL_bmp.c
@@ -407,14 +407,20 @@ SDL_LoadBMP_RW(SDL_RWops * src, int freesrc)
             goto done;
         }
 
-        /*
-        | guich: always use 1<<bpp b/c some bitmaps can bring wrong information
-        | for colorsUsed
-        */
-        /* if (biClrUsed == 0) {  */
-        biClrUsed = 1 << biBitCount;
-        /* } */
-        if (biSize == 12) {
+        if (biClrUsed == 0) {
+            biClrUsed = 1 << biBitCount;
+        }
+
+        if (biClrUsed > palette->ncolors) {
+            biClrUsed = 1 << biBitCount;  /* try forcing it? */
+            if (biClrUsed > palette->ncolors) {
+                SDL_SetError("Unsupported or incorrect biClrUsed field");
+                was_error = SDL_TRUE;
+                goto done;
+            }
+        }
+
+       if (biSize == 12) {
             for (i = 0; i < (int) biClrUsed; ++i) {
                 SDL_RWread(src, &palette->colors[i].b, 1, 1);
                 SDL_RWread(src, &palette->colors[i].g, 1, 1);