Skip to content

Commit 40d97bf

Browse files
committed
CVE-2019-7637: Fix in integer overflow in SDL_CalculatePitch
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>
1 parent 8906afb commit 40d97bf

8 files changed

Lines changed: 57 additions & 7 deletions

File tree

src/video/SDL_pixels.c

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -286,26 +286,53 @@ void SDL_DitherColors(SDL_Color *colors, int bpp)
286286
}
287287
}
288288
/*
289-
* Calculate the pad-aligned scanline width of a surface
289+
* Calculate the pad-aligned scanline width of a surface. Return 0 in case of
290+
* an error.
290291
*/
291292
Uint16 SDL_CalculatePitch(SDL_Surface *surface)
292293
{
293-
Uint16 pitch;
294+
unsigned int pitch = 0;
294295

295296
/* Surface should be 4-byte aligned for speed */
296-
pitch = surface->w*surface->format->BytesPerPixel;
297+
/* The code tries to prevent from an Uint16 overflow. */;
298+
for (Uint8 byte = surface->format->BytesPerPixel; byte; byte--) {
299+
pitch += (unsigned int)surface->w;
300+
if (pitch < surface->w) {
301+
SDL_SetError("A scanline is too wide");
302+
return(0);
303+
}
304+
}
297305
switch (surface->format->BitsPerPixel) {
298306
case 1:
299-
pitch = (pitch+7)/8;
307+
if (pitch % 8) {
308+
pitch = pitch / 8 + 1;
309+
} else {
310+
pitch = pitch / 8;
311+
}
300312
break;
301313
case 4:
302-
pitch = (pitch+1)/2;
314+
if (pitch % 2) {
315+
pitch = pitch / 2 + 1;
316+
} else {
317+
pitch = pitch / 2;
318+
}
303319
break;
304320
default:
305321
break;
306322
}
307-
pitch = (pitch + 3) & ~3; /* 4-byte aligning */
308-
return(pitch);
323+
/* 4-byte aligning */
324+
if (pitch & 3) {
325+
if (pitch + 3 < pitch) {
326+
SDL_SetError("A scanline is too wide");
327+
return(0);
328+
}
329+
pitch = (pitch + 3) & ~3;
330+
}
331+
if (pitch > 0xFFFF) {
332+
SDL_SetError("A scanline is too wide");
333+
return(0);
334+
}
335+
return((Uint16)pitch);
309336
}
310337
/*
311338
* Match an RGB value to a particular palette index

src/video/gapi/SDL_gapivideo.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,9 @@ SDL_Surface *GAPI_SetVideoMode(_THIS, SDL_Surface *current,
733733
video->w = gapi->w = width;
734734
video->h = gapi->h = height;
735735
video->pitch = SDL_CalculatePitch(video);
736+
if (!current->pitch) {
737+
return(NULL);
738+
}
736739

737740
/* Small fix for WinCE/Win32 - when activating window
738741
SDL_VideoSurface is equal to zero, so activating code

src/video/nanox/SDL_nxvideo.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,10 @@ SDL_Surface * NX_SetVideoMode (_THIS, SDL_Surface * current,
378378
current -> w = width ;
379379
current -> h = height ;
380380
current -> pitch = SDL_CalculatePitch (current) ;
381+
if (!current->pitch) {
382+
current = NULL;
383+
goto done;
384+
}
381385
NX_ResizeImage (this, current, flags) ;
382386
}
383387

src/video/ps2gs/SDL_gsvideo.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,9 @@ static SDL_Surface *GS_SetVideoMode(_THIS, SDL_Surface *current,
479479
current->w = width;
480480
current->h = height;
481481
current->pitch = SDL_CalculatePitch(current);
482+
if (!current->pitch) {
483+
return(NULL);
484+
}
482485

483486
/* Memory map the DMA area for block memory transfer */
484487
if ( ! mapped_mem ) {

src/video/ps3/SDL_ps3video.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,9 @@ static SDL_Surface *PS3_SetVideoMode(_THIS, SDL_Surface * current, int width, in
339339
current->w = width;
340340
current->h = height;
341341
current->pitch = SDL_CalculatePitch(current);
342+
if (!current->pitch) {
343+
return(NULL);
344+
}
342345

343346
/* Alloc aligned mem for current->pixels */
344347
s_pixels = memalign(16, current->h * current->pitch);

src/video/windib/SDL_dibvideo.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,9 @@ SDL_Surface *DIB_SetVideoMode(_THIS, SDL_Surface *current,
675675
video->w = width;
676676
video->h = height;
677677
video->pitch = SDL_CalculatePitch(video);
678+
if (!current->pitch) {
679+
return(NULL);
680+
}
678681

679682
/* Small fix for WinCE/Win32 - when activating window
680683
SDL_VideoSurface is equal to zero, so activating code

src/video/windx5/SDL_dx5video.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,6 +1127,9 @@ SDL_Surface *DX5_SetVideoMode(_THIS, SDL_Surface *current,
11271127
video->w = width;
11281128
video->h = height;
11291129
video->pitch = SDL_CalculatePitch(video);
1130+
if (!current->pitch) {
1131+
return(NULL);
1132+
}
11301133

11311134
#ifndef NO_CHANGEDISPLAYSETTINGS
11321135
/* Set fullscreen mode if appropriate.

src/video/x11/SDL_x11video.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,6 +1225,10 @@ SDL_Surface *X11_SetVideoMode(_THIS, SDL_Surface *current,
12251225
current->w = width;
12261226
current->h = height;
12271227
current->pitch = SDL_CalculatePitch(current);
1228+
if (!current->pitch) {
1229+
current = NULL;
1230+
goto done;
1231+
}
12281232
if (X11_ResizeImage(this, current, flags) < 0) {
12291233
current = NULL;
12301234
goto done;

0 commit comments

Comments
 (0)