# HG changeset patch # User Sam Lantinga # Date 1508191062 25200 # Mon Oct 16 14:57:42 2017 -0700 # Node ID 81a4950907a01359f2f9390875291eb3951e6c6b # Parent 97bc026b46ded1ef28709d246130e66e81f1b513 Fixed bug 3890 - Incomplete fix for CVE-2017-2888 Felix Geyer http://hg.libsdl.org/SDL/rev/7e0f1498ddb5 tries to fix CVE-2017-2888. Unfortunately compilers may optimize the second condition "(size / surface->pitch) != surface->h" away. See https://bugzilla.redhat.com/show_bug.cgi?id=1500623#c2 I've verified that this is also the case on Debian unstable (gcc 7.2). diff -r 97bc026b46de -r 81a4950907a0 src/video/SDL_surface.c --- a/src/video/SDL_surface.c Mon Oct 16 14:39:56 2017 -0700 +++ b/src/video/SDL_surface.c Mon Oct 16 14:57:42 2017 -0700 @@ -26,6 +26,10 @@ #include "SDL_RLEaccel_c.h" #include "SDL_pixels_c.h" +/* Check to make sure we can safely check multiplication of surface w and pitch and it won't overflow size_t */ +SDL_COMPILE_TIME_ASSERT(surface_size_assumptions, + sizeof(int) == sizeof(Sint32) && sizeof(size_t) >= sizeof(Sint32)); + /* Public routines */ /* @@ -91,15 +95,16 @@ /* Get the pixels */ if (surface->w && surface->h) { - int size = (surface->h * surface->pitch); - if (size < 0 || (size / surface->pitch) != surface->h) { + /* Assumptions checked in surface_size_assumptions assert above */ + Sint64 size = ((Sint64)surface->h * surface->pitch); + if (size < 0 || size > SDL_MAX_SINT32) { /* Overflow... */ SDL_FreeSurface(surface); SDL_OutOfMemory(); return NULL; } - surface->pixels = SDL_malloc(size); + surface->pixels = SDL_malloc((size_t)size); if (!surface->pixels) { SDL_FreeSurface(surface); SDL_OutOfMemory();