Fixed CVE-2019-7635 and bug 4498 - Heap-Buffer Overflow in Blit1to4 pertaining to SDL_blit_1.c

Petr Pisar

The root cause is that the POC BMP file declares 3 colors used and 4 bpp palette, but pixel at line 28 and column 1 (counted from 0) has color number 3. Then when the image loaded into a surface is passed to SDL_DisplayFormat(), in order to convert it to a video format, a used bliting function looks up a color number 3 in a 3-element long color bliting map. (The map obviously has the same number entries as the surface format has colors.)

Proper fix should refuse broken BMP images that have a pixel with a color index higher than declared number of "used" colors. Possibly more advanced fix could try to relocate the out-of-range color index into a vacant index (if such exists).

src/video/SDL_bmp.c

@@ -246,6 +246,14 @@ SDL_LoadBMP_RW(SDL_RWops * src, int freesrc)
         ExpandBMP = biBitCount;
         biBitCount = 8;
         break;
+    case 2:
+    case 3:
+    case 5:
+    case 6:
+    case 7:
+		SDL_SetError("%d-bpp BMP images are not supported", biBitCount);
+        was_error = SDL_TRUE;
+        goto done;
     default:
         ExpandBMP = 0;
         break;
@@ -398,19 +406,32 @@ SDL_LoadBMP_RW(SDL_RWops * src, int freesrc)
                             goto done;
                         }
                     }
-                    *(bits + i) = (pixel >> shift);
+                    bits[i] = (pixel >> shift);
+					if (bits[i] >= biClrUsed) {
+						SDL_SetError("A BMP image contains a pixel with a color out of the palette");
+						was_error = SDL_TRUE;
+						goto done;
+					}
                     pixel <<= ExpandBMP;
                 }
             }
             break;
 
         default:
-            if (SDL_RWread(src, bits, 1, surface->pitch)
-                != surface->pitch) {
+            if (SDL_RWread(src, bits, 1, surface->pitch) != surface->pitch) {
                 SDL_Error(SDL_EFREAD);
                 was_error = SDL_TRUE;
                 goto done;
             }
+			if (biBitCount == 8 && palette && biClrUsed < (1 << biBitCount)) {
+				for (i = 0; i < surface->w; ++i) {
+					if (bits[i] >= biClrUsed) {
+						SDL_SetError("A BMP image contains a pixel with a color out of the palette");
+						was_error = SDL_TRUE;
+						goto done;
+					}
+				}
+			}
 #if SDL_BYTEORDER == SDL_BIG_ENDIAN
             /* Byte-swap the pixels if needed. Note that the 24bpp
                case has already been taken care of above. */