From 7cafd3e820489f17f86d0d897ad9719ef54599f1 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>

Date: Mon, 18 Feb 2019 13:53:16 +0100

Subject: [PATCH 11/11] CVE-2019-7637: Fix in integer overflow in

 SDL_CalculatePitch

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

If a too large width is passed to SDL_SetVideoMode() the width travels

to SDL_CalculatePitch() where the width (e.g. 65535) is multiplied by

BytesPerPixel (e.g. 4) and the result is stored into Uint16 pitch

variable. During this arithmetics an integer overflow can happen (e.g.

the value is clamped as 65532). As a result SDL_Surface with a pitch

smaller than width * BytesPerPixel is created, too small pixel buffer

is allocated and when the SDL_Surface is processed in SDL_FillRect()

a buffer overflow occurs.

This can be reproduced with "./graywin -width 21312312313123213213213"

command.

This patch fixes is by using a very careful arithmetics in

SDL_CalculatePitch(). If an overflow is detected, an error is reported

back as a special 0 value. We assume that 0-width surfaces do not

occur in the wild. Since SDL_CalculatePitch() is a private function,

we can change the semantics.

CVE-2019-7637

https://bugzilla.libsdl.org/show_bug.cgi?id=4497

Signed-off-by: Petr Písař <ppisar@redhat.com>

---

 src/video/SDL_pixels.c          | 41 +++++++++++++++++++++++++++------

 src/video/gapi/SDL_gapivideo.c  |  3 +++

 src/video/nanox/SDL_nxvideo.c   |  4 ++++

 src/video/ps2gs/SDL_gsvideo.c   |  3 +++

 src/video/ps3/SDL_ps3video.c    |  3 +++

 src/video/windib/SDL_dibvideo.c |  3 +++

 src/video/windx5/SDL_dx5video.c |  3 +++

 src/video/x11/SDL_x11video.c    |  4 ++++

 8 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/src/video/SDL_pixels.c b/src/video/SDL_pixels.c

index 1a7fd518f..44626b749 100644

--- a/src/video/SDL_pixels.c

+++ b/src/video/SDL_pixels.c

@@ -286,26 +286,53 @@ void SDL_DitherColors(SDL_Color *colors, int bpp)

 	}

 }

 /* 

- * Calculate the pad-aligned scanline width of a surface

+ * Calculate the pad-aligned scanline width of a surface. Return 0 in case of

+ * an error.

  */

 Uint16 SDL_CalculatePitch(SDL_Surface *surface)

 {

-	Uint16 pitch;

+	unsigned int pitch = 0;

 	/* Surface should be 4-byte aligned for speed */

-	pitch = surface->w*surface->format->BytesPerPixel;

+	/* The code tries to prevent from an Uint16 overflow. */;

+	for (Uint8 byte = surface->format->BytesPerPixel; byte; byte--) {

+		pitch += (unsigned int)surface->w;

+		if (pitch < surface->w) {

+			SDL_SetError("A scanline is too wide");

+			return(0);

+		}

+	}

 	switch (surface->format->BitsPerPixel) {

 		case 1:

-			pitch = (pitch+7)/8;

+			if (pitch % 8) {

+				pitch = pitch / 8 + 1;

+			} else {

+				pitch = pitch / 8;

+			}

 			break;

 		case 4:

-			pitch = (pitch+1)/2;

+			if (pitch % 2) {

+				pitch = pitch / 2 + 1;

+			} else {

+				pitch = pitch / 2;

+			}

 			break;

 		default:

 			break;

 	}

-	pitch = (pitch + 3) & ~3;	/* 4-byte aligning */

-	return(pitch);

+	/* 4-byte aligning */

+	if (pitch & 3) {

+		if (pitch + 3 < pitch) {

+			SDL_SetError("A scanline is too wide");

+			return(0);

+		}

+		pitch = (pitch + 3) & ~3;

+	}

+	if (pitch > 0xFFFF) {

+		SDL_SetError("A scanline is too wide");

+		return(0);

+	}

+	return((Uint16)pitch);

 }

 /*

  * Match an RGB value to a particular palette index

diff --git a/src/video/gapi/SDL_gapivideo.c b/src/video/gapi/SDL_gapivideo.c

index 86deadc75..8a0648536 100644

--- a/src/video/gapi/SDL_gapivideo.c

+++ b/src/video/gapi/SDL_gapivideo.c

@@ -733,6 +733,9 @@ SDL_Surface *GAPI_SetVideoMode(_THIS, SDL_Surface *current,

 	video->w = gapi->w = width;

 	video->h = gapi->h = height;

 	video->pitch = SDL_CalculatePitch(video); 

+	if (!current->pitch) {

+		return(NULL);

+	}

 	/* Small fix for WinCE/Win32 - when activating window

 	   SDL_VideoSurface is equal to zero, so activating code

diff --git a/src/video/nanox/SDL_nxvideo.c b/src/video/nanox/SDL_nxvideo.c

index b188e0958..cbdd09a08 100644

--- a/src/video/nanox/SDL_nxvideo.c

+++ b/src/video/nanox/SDL_nxvideo.c

@@ -378,6 +378,10 @@ SDL_Surface * NX_SetVideoMode (_THIS, SDL_Surface * current,

         current -> w = width ;

         current -> h = height ;

         current -> pitch = SDL_CalculatePitch (current) ;

+        if (!current->pitch) {

+            current = NULL;

+            goto done;

+        }

         NX_ResizeImage (this, current, flags) ;

     }

diff --git a/src/video/ps2gs/SDL_gsvideo.c b/src/video/ps2gs/SDL_gsvideo.c

index e172c60dc..329086680 100644

--- a/src/video/ps2gs/SDL_gsvideo.c

+++ b/src/video/ps2gs/SDL_gsvideo.c

@@ -479,6 +479,9 @@ static SDL_Surface *GS_SetVideoMode(_THIS, SDL_Surface *current,

 	current->w = width;

 	current->h = height;

 	current->pitch = SDL_CalculatePitch(current);

+	if (!current->pitch) {

+		return(NULL);

+	}

 	/* Memory map the DMA area for block memory transfer */

 	if ( ! mapped_mem ) {

diff --git a/src/video/ps3/SDL_ps3video.c b/src/video/ps3/SDL_ps3video.c

index d5519e051..17848e33a 100644

--- a/src/video/ps3/SDL_ps3video.c

+++ b/src/video/ps3/SDL_ps3video.c

@@ -339,6 +339,9 @@ static SDL_Surface *PS3_SetVideoMode(_THIS, SDL_Surface * current, int width, in

 	current->w = width;

 	current->h = height;

 	current->pitch = SDL_CalculatePitch(current);

+	if (!current->pitch) {

+		return(NULL);

+	}

 	/* Alloc aligned mem for current->pixels */

 	s_pixels = memalign(16, current->h * current->pitch);

diff --git a/src/video/windib/SDL_dibvideo.c b/src/video/windib/SDL_dibvideo.c

index 6187bfcf7..86ebb12a3 100644

--- a/src/video/windib/SDL_dibvideo.c

+++ b/src/video/windib/SDL_dibvideo.c

@@ -675,6 +675,9 @@ SDL_Surface *DIB_SetVideoMode(_THIS, SDL_Surface *current,

 	video->w = width;

 	video->h = height;

 	video->pitch = SDL_CalculatePitch(video);

+	if (!current->pitch) {

+		return(NULL);

+	}

 	/* Small fix for WinCE/Win32 - when activating window

 	   SDL_VideoSurface is equal to zero, so activating code

diff --git a/src/video/windx5/SDL_dx5video.c b/src/video/windx5/SDL_dx5video.c

index f80ca97b0..39fc4fc37 100644

--- a/src/video/windx5/SDL_dx5video.c

+++ b/src/video/windx5/SDL_dx5video.c

@@ -1127,6 +1127,9 @@ SDL_Surface *DX5_SetVideoMode(_THIS, SDL_Surface *current,

 		video->w = width;

 		video->h = height;

 		video->pitch = SDL_CalculatePitch(video);

+		if (!current->pitch) {

+			return(NULL);

+		}

 #ifndef NO_CHANGEDISPLAYSETTINGS

 		/* Set fullscreen mode if appropriate.

diff --git a/src/video/x11/SDL_x11video.c b/src/video/x11/SDL_x11video.c

index 79e60f971..45d1f79be 100644

--- a/src/video/x11/SDL_x11video.c

+++ b/src/video/x11/SDL_x11video.c

@@ -1220,6 +1220,10 @@ SDL_Surface *X11_SetVideoMode(_THIS, SDL_Surface *current,

 		current->w = width;

 		current->h = height;

 		current->pitch = SDL_CalculatePitch(current);

+		if (!current->pitch) {

+			current = NULL;

+			goto done;

+		}

 		if (X11_ResizeImage(this, current, flags) < 0) {

 			current = NULL;

 			goto done;

-- 

2.21.0