From 775eac98ee373bc7abdf0eb773534f5a7b44f42d Mon Sep 17 00:00:00 2001
From: Anonymous Maarten <[EMAIL REDACTED]>
Date: Sun, 1 Feb 2026 22:22:59 +0100
Subject: [PATCH] surface: verify surface palette in SDL_Save(BMP|PNG) before
hitting the FS
(cherry picked from commit 28ea4a8e319165f045b4ee9c531bef7360b4746f)
---
src/video/SDL_bmp.c | 194 ++++++++++++++++++++--------------
src/video/SDL_stb.c | 10 +-
test/testautomation_surface.c | 29 +++++
3 files changed, 154 insertions(+), 79 deletions(-)
diff --git a/src/video/SDL_bmp.c b/src/video/SDL_bmp.c
index 118bf11d356d1..de1db2e986f5a 100644
--- a/src/video/SDL_bmp.c
+++ b/src/video/SDL_bmp.c
@@ -591,6 +591,78 @@ SDL_Surface *SDL_LoadBMP_IO(SDL_IOStream *src, bool closeio)
return surface;
}
+typedef struct {
+ SDL_Surface *surface;
+ SDL_Surface *intermediate_surface;
+ bool save32bit;
+ bool saveLegacyBMP;
+} BMPSaveState;
+
+static void FreeBMPSaveState(BMPSaveState *state)
+{
+ if (state->intermediate_surface && state->intermediate_surface != state->surface) {
+ SDL_DestroySurface(state->intermediate_surface);
+ }
+}
+
+static bool InitBMPSaveState(BMPSaveState *state, SDL_Surface *surface)
+{
+ state->surface = surface;
+ state->intermediate_surface = NULL;
+ state->save32bit = false;
+ state->saveLegacyBMP = false;
+
+ CHECK_PARAM(!SDL_SurfaceValid(surface)) {
+ return SDL_InvalidParamError("surface");
+ }
+
+#ifdef SAVE_32BIT_BMP
+ // We can save alpha information in a 32-bit BMP
+ if (SDL_BITSPERPIXEL(surface->format) >= 8 &&
+ (SDL_ISPIXELFORMAT_ALPHA(surface->format) ||
+ (surface->map.info.flags & SDL_COPY_COLORKEY))) {
+ state->save32bit = true;
+ }
+#endif // SAVE_32BIT_BMP
+
+ if (surface->palette && !state->save32bit) {
+ if (SDL_BITSPERPIXEL(surface->format) == 8) {
+ state->intermediate_surface = surface;
+ } else {
+ SDL_SetError("%u bpp BMP files not supported",
+ SDL_BITSPERPIXEL(surface->format));
+ goto error;
+ }
+ } else if ((surface->format == SDL_PIXELFORMAT_BGR24 && !state->save32bit) ||
+ (surface->format == SDL_PIXELFORMAT_BGRA32 && state->save32bit)) {
+ state->intermediate_surface = surface;
+ } else {
+ SDL_PixelFormat pixel_format;
+
+ /* If the surface has a colorkey or alpha channel we'll save a
+ 32-bit BMP with alpha channel, otherwise save a 24-bit BMP. */
+ if (state->save32bit) {
+ pixel_format = SDL_PIXELFORMAT_BGRA32;
+ } else {
+ pixel_format = SDL_PIXELFORMAT_BGR24;
+ }
+ state->intermediate_surface = SDL_ConvertSurface(surface, pixel_format);
+ if (!state->intermediate_surface) {
+ SDL_SetError("Couldn't convert image to %d bpp",
+ (int)SDL_BITSPERPIXEL(pixel_format));
+ goto error;
+ }
+ }
+
+ if (state->save32bit) {
+ state->saveLegacyBMP = SDL_GetHintBoolean(SDL_HINT_BMP_SAVE_LEGACY_FORMAT, false);
+ }
+ return true;
+error:
+ FreeBMPSaveState(state);
+ return false;
+}
+
SDL_Surface *SDL_LoadBMP(const char *file)
{
SDL_IOStream *stream = SDL_IOFromFile(file, "rb");
@@ -599,16 +671,12 @@ SDL_Surface *SDL_LoadBMP(const char *file)
}
return SDL_LoadBMP_IO(stream, true);
}
-
-bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio)
+static bool SDL_SaveBMP_IO_Internal(BMPSaveState *state, SDL_IOStream *dst, bool closeio)
{
bool was_error = true;
Sint64 fp_offset, new_offset;
int i, pad;
- SDL_Surface *intermediate_surface = NULL;
Uint8 *bits;
- bool save32bit = false;
- bool saveLegacyBMP = false;
// The Win32 BMP file header (14 bytes)
char magic[2] = { 'B', 'M' };
@@ -647,60 +715,8 @@ bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio)
Uint32 bV5ProfileSize = 0;
Uint32 bV5Reserved = 0;
- // Make sure we have somewhere to save
- CHECK_PARAM(!SDL_SurfaceValid(surface)) {
- SDL_InvalidParamError("surface");
- goto done;
- }
- CHECK_PARAM(!dst) {
- SDL_InvalidParamError("dst");
- goto done;
- }
-
-#ifdef SAVE_32BIT_BMP
- // We can save alpha information in a 32-bit BMP
- if (SDL_BITSPERPIXEL(surface->format) >= 8 &&
- (SDL_ISPIXELFORMAT_ALPHA(surface->format) ||
- surface->map.info.flags & SDL_COPY_COLORKEY)) {
- save32bit = true;
- }
-#endif // SAVE_32BIT_BMP
-
- if (surface->palette && !save32bit) {
- if (SDL_BITSPERPIXEL(surface->format) == 8) {
- intermediate_surface = surface;
- } else {
- SDL_SetError("%u bpp BMP files not supported",
- SDL_BITSPERPIXEL(surface->format));
- goto done;
- }
- } else if ((surface->format == SDL_PIXELFORMAT_BGR24 && !save32bit) ||
- (surface->format == SDL_PIXELFORMAT_BGRA32 && save32bit)) {
- intermediate_surface = surface;
- } else {
- SDL_PixelFormat pixel_format;
-
- /* If the surface has a colorkey or alpha channel we'll save a
- 32-bit BMP with alpha channel, otherwise save a 24-bit BMP. */
- if (save32bit) {
- pixel_format = SDL_PIXELFORMAT_BGRA32;
- } else {
- pixel_format = SDL_PIXELFORMAT_BGR24;
- }
- intermediate_surface = SDL_ConvertSurface(surface, pixel_format);
- if (!intermediate_surface) {
- SDL_SetError("Couldn't convert image to %d bpp",
- (int)SDL_BITSPERPIXEL(pixel_format));
- goto done;
- }
- }
-
- if (save32bit) {
- saveLegacyBMP = SDL_GetHintBoolean(SDL_HINT_BMP_SAVE_LEGACY_FORMAT, false);
- }
-
- if (SDL_LockSurface(intermediate_surface)) {
- const size_t bw = intermediate_surface->w * intermediate_surface->fmt->bytes_per_pixel;
+ if (SDL_LockSurface(state->intermediate_surface)) {
+ const size_t bw = state->intermediate_surface->w * state->intermediate_surface->fmt->bytes_per_pixel;
// Set the BMP file header values
bfSize = 0; // We'll write this when we're done
@@ -723,23 +739,23 @@ bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio)
// Set the BMP info values
biSize = 40;
- biWidth = intermediate_surface->w;
- biHeight = intermediate_surface->h;
+ biWidth = state->intermediate_surface->w;
+ biHeight = state->intermediate_surface->h;
biPlanes = 1;
- biBitCount = intermediate_surface->fmt->bits_per_pixel;
+ biBitCount = state->intermediate_surface->fmt->bits_per_pixel;
biCompression = BI_RGB;
- biSizeImage = intermediate_surface->h * intermediate_surface->pitch;
+ biSizeImage = state->intermediate_surface->h * state->intermediate_surface->pitch;
biXPelsPerMeter = 0;
biYPelsPerMeter = 0;
- if (intermediate_surface->palette) {
- biClrUsed = intermediate_surface->palette->ncolors;
+ if (state->intermediate_surface->palette) {
+ biClrUsed = state->intermediate_surface->palette->ncolors;
} else {
biClrUsed = 0;
}
biClrImportant = 0;
// Set the BMP info values
- if (save32bit && !saveLegacyBMP) {
+ if (state->save32bit && !state->saveLegacyBMP) {
biSize = 124;
// Version 4 values
biCompression = BI_BITFIELDS;
@@ -775,7 +791,7 @@ bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio)
}
// Write the BMP info values
- if (save32bit && !saveLegacyBMP) {
+ if (state->save32bit && !state->saveLegacyBMP) {
// Version 4 values
if (!SDL_WriteU32LE(dst, bV4RedMask) ||
!SDL_WriteU32LE(dst, bV4GreenMask) ||
@@ -804,12 +820,12 @@ bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio)
}
// Write the palette (in BGR color order)
- if (intermediate_surface->palette) {
+ if (state->intermediate_surface->palette) {
SDL_Color *colors;
int ncolors;
- colors = intermediate_surface->palette->colors;
- ncolors = intermediate_surface->palette->ncolors;
+ colors = state->intermediate_surface->palette->colors;
+ ncolors = state->intermediate_surface->palette->ncolors;
for (i = 0; i < ncolors; ++i) {
if (!SDL_WriteU8(dst, colors[i].b) ||
!SDL_WriteU8(dst, colors[i].g) ||
@@ -833,10 +849,10 @@ bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio)
}
// Write the bitmap image upside down
- bits = (Uint8 *)intermediate_surface->pixels + (intermediate_surface->h * intermediate_surface->pitch);
+ bits = (Uint8 *)state->intermediate_surface->pixels + (state->intermediate_surface->h * state->intermediate_surface->pitch);
pad = ((bw % 4) ? (4 - (bw % 4)) : 0);
- while (bits > (Uint8 *)intermediate_surface->pixels) {
- bits -= intermediate_surface->pitch;
+ while (bits > (Uint8 *)state->intermediate_surface->pixels) {
+ bits -= state->intermediate_surface->pitch;
if (SDL_WriteIO(dst, bits, bw) != bw) {
goto done;
}
@@ -867,15 +883,12 @@ bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio)
}
// Close it up..
- SDL_UnlockSurface(intermediate_surface);
+ SDL_UnlockSurface(state->intermediate_surface);
was_error = false;
}
done:
- if (intermediate_surface && intermediate_surface != surface) {
- SDL_DestroySurface(intermediate_surface);
- }
if (closeio && dst) {
if (!SDL_CloseIO(dst)) {
was_error = true;
@@ -887,11 +900,36 @@ bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio)
return true;
}
+bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio)
+{
+ bool result = true;
+ BMPSaveState state;
+ if (!InitBMPSaveState(&state, surface)) {
+ return false;
+ }
+ CHECK_PARAM(!dst) {
+ result = SDL_InvalidParamError("dst");
+ goto done;
+ }
+ result = SDL_SaveBMP_IO_Internal(&state, dst, closeio);
+done:
+ FreeBMPSaveState(&state);
+ return result;
+}
+
bool SDL_SaveBMP(SDL_Surface *surface, const char *file)
{
+ BMPSaveState state;
+ if (!InitBMPSaveState(&state, surface)) {
+ return false;
+ }
+
SDL_IOStream *stream = SDL_IOFromFile(file, "wb");
if (!stream) {
return false;
}
- return SDL_SaveBMP_IO(surface, stream, true);
+
+ bool result = SDL_SaveBMP_IO_Internal(&state, stream, true);
+ FreeBMPSaveState(&state);
+ return result;
}
diff --git a/src/video/SDL_stb.c b/src/video/SDL_stb.c
index 6748c9c598ba6..a4b0a718ab05b 100644
--- a/src/video/SDL_stb.c
+++ b/src/video/SDL_stb.c
@@ -395,7 +395,7 @@ bool SDL_SavePNG_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio)
Uint8 *trns = NULL;
bool free_surface = false;
- // Make sure we have somewhere to save
+ // Make sure we have something to save
CHECK_PARAM(!SDL_SurfaceValid(surface)) {
SDL_InvalidParamError("surface");
goto done;
@@ -478,6 +478,14 @@ bool SDL_SavePNG_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio)
bool SDL_SavePNG(SDL_Surface *surface, const char *file)
{
#ifdef SDL_HAVE_STB
+ // Make sure we have something to save
+ CHECK_PARAM(!SDL_SurfaceValid(surface)) {
+ return SDL_InvalidParamError("surface");
+ }
+
+ if (SDL_ISPIXELFORMAT_INDEXED(surface->format) && !surface->palette) {
+ return SDL_SetError("Indexed surfaces must have a palette");
+ }
SDL_IOStream *stream = SDL_IOFromFile(file, "wb");
if (!stream) {
return false;
diff --git a/test/testautomation_surface.c b/test/testautomation_surface.c
index bb4e3e07443fb..6061cf0c4919d 100644
--- a/test/testautomation_surface.c
+++ b/test/testautomation_surface.c
@@ -328,6 +328,7 @@ static int SDLCALL surface_testSaveLoad(void *arg)
int ret;
const char *sampleFilename = "testSaveLoad.tmp";
SDL_Surface *face;
+ SDL_Surface *indexed_surface;
SDL_Surface *rface;
SDL_Palette *palette;
SDL_Color colors[] = {
@@ -344,6 +345,18 @@ static int SDLCALL surface_testSaveLoad(void *arg)
return TEST_ABORTED;
}
+ indexed_surface = SDL_CreateSurface(32, 32, SDL_PIXELFORMAT_INDEX8);
+ SDLTest_AssertCheck(indexed_surface != NULL, "SDL_CreateSurface(SDL_PIXELFORMAT_INDEX8)");
+
+ /* Delete test file; ignore errors */
+ SDL_RemovePath(sampleFilename);
+
+ /* Saving an indexed surface without palette as BMP fails */
+ ret = SDL_SaveBMP(indexed_surface, sampleFilename);
+ SDLTest_AssertPass("Call to SDL_SaveBMP() using an indexed surface without palette");
+ SDLTest_AssertCheck(ret == false, "Verify result of SDL_SaveBMP(indexed_surface without palette), expected: false, got: %i", ret);
+ SDLTest_AssertCheck(!SDL_GetPathInfo(sampleFilename, NULL), "No file is created after trying to save a indexed surface without palette");
+
/* Delete test file; ignore errors */
SDL_RemovePath(sampleFilename);
@@ -367,6 +380,15 @@ static int SDLCALL surface_testSaveLoad(void *arg)
/* Delete test file; ignore errors */
SDL_RemovePath(sampleFilename);
+ /* Saving an indexed surface as PNG fails */
+ ret = SDL_SavePNG(indexed_surface, sampleFilename);
+ SDLTest_AssertPass("Call to SDL_SavePNG() using an indexed surface without palette");
+ SDLTest_AssertCheck(ret == false, "Verify result of SDL_SavePNG(indexed surface without palette), expected: false, got: %i", ret);
+ SDLTest_AssertCheck(ret == false, "Verify result of SDL_SavePNG(indexed surface without palette), expected: false, got: %i", ret);
+
+ /* Delete test file; ignore errors */
+ SDL_RemovePath(sampleFilename);
+
/* Save a PNG surface */
ret = SDL_SavePNG(face, sampleFilename);
SDLTest_AssertPass("Call to SDL_SavePNG()");
@@ -416,6 +438,9 @@ static int SDLCALL surface_testSaveLoad(void *arg)
if (stream == NULL) {
return TEST_ABORTED;
}
+ ret = SDL_SaveBMP_IO(indexed_surface, stream, false);
+ SDLTest_AssertCheck(ret == false, "Verify result from SDL_SaveBMP (indexed surface without palette), expected: false, got: %i", ret);
+ SDL_SeekIO(stream, 0, SDL_IO_SEEK_SET);
ret = SDL_SaveBMP_IO(face, stream, false);
SDLTest_AssertPass("Call to SDL_SaveBMP()");
SDLTest_AssertCheck(ret == true, "Verify result from SDL_SaveBMP, expected: true, got: %i", ret);
@@ -447,6 +472,9 @@ static int SDLCALL surface_testSaveLoad(void *arg)
if (stream == NULL) {
return TEST_ABORTED;
}
+ ret = SDL_SavePNG_IO(indexed_surface, stream, false);
+ SDLTest_AssertCheck(ret == false, "Verify result from SDL_SavePNG_IO (indexed surface without palette), expected: false, got: %i", ret);
+ SDL_SeekIO(stream, 0, SDL_IO_SEEK_SET);
ret = SDL_SavePNG_IO(face, stream, false);
SDLTest_AssertPass("Call to SDL_SavePNG()");
SDLTest_AssertCheck(ret == true, "Verify result from SDL_SavePNG, expected: true, got: %i", ret);
@@ -472,6 +500,7 @@ static int SDLCALL surface_testSaveLoad(void *arg)
SDL_CloseIO(stream);
stream = NULL;
+ SDL_DestroySurface(indexed_surface);
SDL_DestroySurface(face);
return TEST_COMPLETED;