Commit 8c9f7104e32ef8c425623d12ef6825e26b99aec2
2022-05-10T10:33:54
video: Harden calculation of SDL_surface pitch and size against overflow If the width is sufficiently ludicrous, then the calculated pitch or the image size could conceivably be a signed integer overflow, which is undefined behaviour. Calculate in the unsigned size_t domain, with overflow checks. Signed-off-by: Simon McVittie <smcv@collabora.com>
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94
diff --git a/src/video/SDL_surface.c b/src/video/SDL_surface.c
index ec981a2..cc20856 100644
--- a/src/video/SDL_surface.c
+++ b/src/video/SDL_surface.c
@@ -33,22 +33,37 @@
SDL_COMPILE_TIME_ASSERT(surface_size_assumptions,
sizeof(int) == sizeof(Sint32) && sizeof(size_t) >= sizeof(Sint32));
+SDL_COMPILE_TIME_ASSERT(can_indicate_overflow, SDL_SIZE_MAX > SDL_MAX_SINT32);
+
/* Public routines */
/*
- * Calculate the pad-aligned scanline width of a surface
+ * Calculate the pad-aligned scanline width of a surface.
+ * Return SDL_SIZE_MAX on overflow.
*/
-static Sint64
-SDL_CalculatePitch(Uint32 format, int width)
+static size_t
+SDL_CalculatePitch(Uint32 format, size_t width)
{
- Sint64 pitch;
+ size_t pitch;
if (SDL_ISPIXELFORMAT_FOURCC(format) || SDL_BITSPERPIXEL(format) >= 8) {
- pitch = ((Sint64)width * SDL_BYTESPERPIXEL(format));
+ if (SDL_size_mul_overflow(width, SDL_BYTESPERPIXEL(format), &pitch)) {
+ return SDL_SIZE_MAX;
+ }
} else {
- pitch = (((Sint64)width * SDL_BITSPERPIXEL(format)) + 7) / 8;
+ if (SDL_size_mul_overflow(width, SDL_BITSPERPIXEL(format), &pitch)) {
+ return SDL_SIZE_MAX;
+ }
+ if (SDL_size_add_overflow(pitch, 7, &pitch)) {
+ return SDL_SIZE_MAX;
+ }
+ pitch /= 8;
+ }
+ /* 4-byte aligning for speed */
+ if (SDL_size_add_overflow(pitch, 3, &pitch)) {
+ return SDL_SIZE_MAX;
}
- pitch = (pitch + 3) & ~3; /* 4-byte aligning for speed */
+ pitch &= ~3;
return pitch;
}
@@ -60,14 +75,14 @@ SDL_Surface *
SDL_CreateRGBSurfaceWithFormat(Uint32 flags, int width, int height, int depth,
Uint32 format)
{
- Sint64 pitch;
+ size_t pitch;
SDL_Surface *surface;
/* The flags are no longer used, make the compiler happy */
(void)flags;
pitch = SDL_CalculatePitch(format, width);
- if (pitch < 0 || pitch > SDL_MAX_SINT32) {
+ if (pitch > SDL_MAX_SINT32) {
/* Overflow... */
SDL_OutOfMemory();
return NULL;
@@ -113,15 +128,15 @@ SDL_CreateRGBSurfaceWithFormat(Uint32 flags, int width, int height, int depth,
/* Get the pixels */
if (surface->w && surface->h) {
/* Assumptions checked in surface_size_assumptions assert above */
- Sint64 size = ((Sint64)surface->h * surface->pitch);
- if (size < 0 || size > SDL_MAX_SINT32) {
+ size_t size;
+ if (SDL_size_mul_overflow(surface->h, surface->pitch, &size)) {
/* Overflow... */
SDL_FreeSurface(surface);
SDL_OutOfMemory();
return NULL;
}
- surface->pixels = SDL_SIMDAlloc((size_t)size);
+ surface->pixels = SDL_SIMDAlloc(size);
if (!surface->pixels) {
SDL_FreeSurface(surface);
SDL_OutOfMemory();
@@ -129,7 +144,7 @@ SDL_CreateRGBSurfaceWithFormat(Uint32 flags, int width, int height, int depth,
}
surface->flags |= SDL_SIMD_ALIGNED;
/* This is important for bitmaps */
- SDL_memset(surface->pixels, 0, surface->h * surface->pitch);
+ SDL_memset(surface->pixels, 0, size);
}
/* Allocate an empty mapping */