211 lines
6.8 KiB
Diff
211 lines
6.8 KiB
Diff
|
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
|
||
|
|