Protect some DIB functions from bad inputs.
Rein Klazes
wijn at wanadoo.nl
Wed Apr 12 05:00:03 CDT 2006
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();
}
More information about the wine-devel
mailing list