Protect some DIB functions from bad inputs.

[Rein Klazes]

On Tue, 11 Apr 2006 10:24:14 +0200, you wrote:

>> Hi,
>> 
>> There are a couple of entries in the bug database (at least #4334 and
>> #4664) where the application calculates a wrong pointer for bitmap data.
>> The application survives on Windows but crashes on wine.
>> 
>> 
>> Changelog:
>> dlls/x11drv	: dib.c
>> dlls/gdi/tests	: bitmap.c
>> Protect the Set/Stretch DIBitfunctions from accessing bad bitmap data.
>> Add some tests that would crash without that.
>> 
>
>Hello,
>
>I'm sorry if writing to you directly is not right thing to do, I was
>advised so on #wine-hq.
>
>I would just like to ask you why this patch didn't get merged to git
>(and subsequently to 0.9.11) and if there is anything I can do to get it
>in in time for 0.9.12?

I have no idea why it was not merged, never got any comments. Cc' ed to
the developers list for suggestions. A re-diffed patch is attached.

Rein 
-------------- next part --------------
--- wine/dlls/x11drv/dib.c	2006-03-03 15:48:11.000000000 +0100
+++ mywine/dlls/x11drv/dib.c	2006-04-12 10:14:05.000000000 +0200
@@ -3817,6 +3817,7 @@ INT X11DRV_SetDIBitsToDevice( X11DRV_PDE
     LONG width, height;
     BOOL top_down;
     POINT pt;
+    int nrsrcbytes, dibpitch;
 
     if (DIB_GetBitmapInfo( &info->bmiHeader, &width, &height,
 			   &descr.infoBpp, &descr.compression ) == -1)
@@ -3864,6 +3865,16 @@ INT X11DRV_SetDIBitsToDevice( X11DRV_PDE
     if (xSrc + cx >= width) cx = width - xSrc;
     if (!cx || !cy) return lines;
 
+    /* pointer check */
+    dibpitch  = ((width * descr.infoBpp + 31) &~31) / 8;
+    if( descr.compression)
+        nrsrcbytes = 1;
+    else {
+        nrsrcbytes = lines * dibpitch;
+        if( nrsrcbytes < 0) nrsrcbytes = - nrsrcbytes;
+    }
+    if( IsBadReadPtr( bits, nrsrcbytes)) return 0;
+
     /* Update the pixmap from the DIB section */
     X11DRV_LockDIBSection(physDev, DIB_Status_GdiMod, FALSE);
 
@@ -3916,7 +3927,7 @@ INT X11DRV_SetDIBitsToDevice( X11DRV_PDE
     descr.width     = cx;
     descr.height    = cy;
     descr.useShm    = FALSE;
-    descr.dibpitch  = ((width * descr.infoBpp + 31) &~31) / 8;
+    descr.dibpitch  = dibpitch;
 
     result = X11DRV_DIB_SetImageBits( &descr );
 
@@ -3940,6 +3951,7 @@ INT X11DRV_SetDIBits( X11DRV_PDEVICE *ph
   BITMAP bitmap;
   LONG width, height, tmpheight;
   INT result;
+  int nrsrcbytes, dibpitch;
 
   descr.physDev = physDev;
 
@@ -3958,6 +3970,16 @@ INT X11DRV_SetDIBits( X11DRV_PDEVICE *ph
 
   if (startscan + lines > height) lines = height - startscan;
 
+  /* pointer check */
+  dibpitch  = ((width * descr.infoBpp + 31) &~31) / 8;
+  if( descr.compression)
+      nrsrcbytes = 1;
+  else {
+      nrsrcbytes = lines * dibpitch;
+      if( nrsrcbytes < 0) nrsrcbytes = - nrsrcbytes;
+  }
+  if( IsBadReadPtr( bits, nrsrcbytes)) return 0;
+
   switch (descr.infoBpp)
   {
        case 1:
@@ -4004,7 +4026,7 @@ INT X11DRV_SetDIBits( X11DRV_PDEVICE *ph
   descr.width     = bitmap.bmWidth;
   descr.height    = lines;
   descr.useShm    = FALSE;
-  descr.dibpitch  = ((descr.infoWidth * descr.infoBpp + 31) &~31) / 8;
+  descr.dibpitch  = dibpitch;
   X11DRV_DIB_Lock( physBitmap, DIB_Status_GdiMod, FALSE );
   result = X11DRV_DIB_SetImageBits( &descr );
   X11DRV_DIB_Unlock( physBitmap, TRUE );
--- wine/dlls/gdi/tests/bitmap.c	2006-04-12 09:43:23.000000000 +0200
+++ mywine/dlls/gdi/tests/bitmap.c	2006-04-12 10:14:05.000000000 +0200
@@ -1001,6 +1001,58 @@ static void test_bitmap(void)
     DeleteDC(hdc);
 }
 
+static void test_badbits()
+{
+    char bmibuf[sizeof(BITMAPINFO) + 256 * sizeof(RGBQUAD)];
+    BITMAPINFO *pbmi = (BITMAPINFO *)bmibuf;
+    HDC hdc0, hdc;
+    int ret;
+    void *bits;
+    DWORD oldprotect;
+    SYSTEM_INFO si;
+    
+    memset(pbmi, 0, sizeof(bmibuf));
+    pbmi->bmiHeader.biSize = sizeof(pbmi->bmiHeader);
+    pbmi->bmiHeader.biHeight = 100;
+    pbmi->bmiHeader.biWidth = 100;
+    pbmi->bmiHeader.biBitCount = 24;
+    pbmi->bmiHeader.biPlanes = 1;
+    pbmi->bmiHeader.biCompression = BI_RGB;
+    hdc0 = GetDC(0);
+    hdc = CreateCompatibleDC( hdc0);
+
+    GetSystemInfo( &si);
+    bits = VirtualAlloc( NULL, 40000, MEM_COMMIT, PAGE_READONLY);
+    ok( (int)bits, "VirtualAlloc failed\n");
+    /* source bits can be read, StretchDIBits succeeds */
+    ret = StretchDIBits( hdc, 0, 0, 100, 100, 0, 0, 100, 100, bits,
+        pbmi, 0, SRCCOPY);
+    ok( ret, "StretchDIBits failed\n");
+    ret = VirtualProtect( (char*)bits + si.dwPageSize, si.dwPageSize,
+        PAGE_NOACCESS, &oldprotect);
+    ok( ret, "VirtualProtect failed\n");
+    /* source bits cannot all be read, StretchDIBits fails */
+    /* and should not crash */
+    ret = StretchDIBits( hdc, 0, 0, 100, 100, 0, 0, 100, 100, bits,
+        pbmi, 0, SRCCOPY);
+    todo_wine {
+        ok( !ret, "StretchDIBits should have failed\n");
+    }
+    /* same tests for SetDIBitsToDevice */
+    ret = SetDIBitsToDevice( hdc, 0, 0, 100, 100, 0, 0, 0, 100, bits,
+        pbmi, DIB_RGB_COLORS);
+    ok( !ret, "SetDIBitsToDevice should have failed\n");
+    ret = VirtualProtect( (char*)bits + si.dwPageSize, si.dwPageSize,
+        PAGE_READONLY, &oldprotect);
+    ok( ret, "VirtualProtect failed\n");
+    ret = SetDIBitsToDevice( hdc, 0, 0, 100, 100, 0, 0, 0, 100, bits,
+        pbmi, DIB_RGB_COLORS);
+    ok( ret, "SetDIBitsToDevice should have succeeded\n");
+
+    DeleteDC( hdc);
+    ReleaseDC(0, hdc0);
+}
+
 START_TEST(bitmap)
 {
     is_win9x = GetWindowLongPtrW(GetDesktopWindow(), GWLP_WNDPROC) == 0;
@@ -1009,4 +1061,5 @@ START_TEST(bitmap)
     test_dibsections();
     test_mono_dibsection();
     test_bitmap();
+    test_badbits();
 }