Fixed bug 3890 - Incomplete fix for CVE-2017-2888
authorSam Lantinga <slouken@libsdl.org>
Mon, 16 Oct 2017 14:57:42 -0700
changeset 1162881a4950907a0
parent 11627 97bc026b46de
child 11629 ec9b9e71f51b
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).
src/video/SDL_surface.c
     1.1 --- a/src/video/SDL_surface.c	Mon Oct 16 14:39:56 2017 -0700
     1.2 +++ b/src/video/SDL_surface.c	Mon Oct 16 14:57:42 2017 -0700
     1.3 @@ -37,6 +37,10 @@
     1.4          const void *src, int src_pitch,
     1.5          Uint32 dst_format, void *dst);
     1.6  
     1.7 +/* Check to make sure we can safely check multiplication of surface w and pitch and it won't overflow size_t */
     1.8 +SDL_COMPILE_TIME_ASSERT(surface_size_assumptions,
     1.9 +    sizeof(int) == sizeof(Sint32) && sizeof(size_t) >= sizeof(Sint32));
    1.10 +
    1.11  /* Public routines */
    1.12  
    1.13  /*
    1.14 @@ -91,15 +95,16 @@
    1.15  
    1.16      /* Get the pixels */
    1.17      if (surface->w && surface->h) {
    1.18 -        int size = (surface->h * surface->pitch);
    1.19 -        if (size < 0 || (size / surface->pitch) != surface->h) {
    1.20 +        /* Assumptions checked in surface_size_assumptions assert above */
    1.21 +        Sint64 size = ((Sint64)surface->h * surface->pitch);
    1.22 +        if (size < 0 || size > SDL_MAX_SINT32) {
    1.23              /* Overflow... */
    1.24              SDL_FreeSurface(surface);
    1.25              SDL_OutOfMemory();
    1.26              return NULL;
    1.27          }
    1.28  
    1.29 -        surface->pixels = SDL_malloc(size);
    1.30 +        surface->pixels = SDL_malloc((size_t)size);
    1.31          if (!surface->pixels) {
    1.32              SDL_FreeSurface(surface);
    1.33              SDL_OutOfMemory();