From 8c9f7104e32ef8c425623d12ef6825e26b99aec2 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 10 May 2022 10:33:54 +0100
Subject: [PATCH] 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>
---
 src/video/SDL_surface.c | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/src/video/SDL_surface.c b/src/video/SDL_surface.c
index ec981a29c..cc2085689 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;
     }
-    pitch = (pitch + 3) & ~3;   /* 4-byte aligning for speed */
+    /* 4-byte aligning for speed */
+    if (SDL_size_add_overflow(pitch, 3, &pitch)) {
+        return SDL_SIZE_MAX;
+    }
+    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 */