Re: [ros-dev] [ros-diffs] [tkreuzer] 70464: [WIN32K] Properly check the bitmap size in IntSetDIBits, taking into account that it might be set to 0, then we have to calculate it ourselves. Fixes a crash when invalid data is ...

[Thomas Faber]

Timo, this is causing testbot to freak out since it now crashes during
an apitest. Could you look into it?
Thanks.

On 2015-12-29 21:28, [e-mail] wrote:
> Author: tkreuzer
> Date: Tue Dec 29 20:28:23 2015
> New Revision: 70464
>
> URL: http://svn.reactos.org/svn/reactos?rev=70464&view=rev
> Log:
> [WIN32K]
> Properly check the bitmap size in IntSetDIBits, taking into account that it might be set to 0, then we have to calculate it ourselves. Fixes a crash when invalid data is passed to NtGdiCreateDIBitmapInternal.
> [GDI32]
> Convert COREINFOHEADER to BITMAPINFOHEADER before passing it to NtGdiCreateDIBitmapInternal, which doesn't support it.
> CORE-10583 #resolve
>
> Modified:
>     trunk/reactos/win32ss/gdi/gdi32/objects/bitmap.c
>     trunk/reactos/win32ss/gdi/ntgdi/dibobj.c
>
> Modified: trunk/reactos/win32ss/gdi/gdi32/objects/bitmap.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/gdi/gdi32/objects/b...
> ==============================================================================
> --- trunk/reactos/win32ss/gdi/gdi32/objects/bitmap.c	[iso-8859-1] (original)
> +++ trunk/reactos/win32ss/gdi/gdi32/objects/bitmap.c	[iso-8859-1] Tue Dec 29 20:28:23 2015
> @@ -408,23 +408,30 @@
>  //  PDC_ATTR pDc_Attr;
>      UINT InfoSize = 0;
>      UINT cjBmpScanSize = 0;
> HBITMAP hBmp;
>
> HBITMAP hBmp = NULL;
> NTSTATUS Status = STATUS_SUCCESS;
>
> PBITMAPINFO pbmiConverted;
>
> UINT cjInfoSize;
>
> /* Convert the BITMAPINFO if it is a COREINFO */
>
> pbmiConverted = ConvertBitmapInfo(Data, ColorUse, &cjInfoSize, FALSE);
>
> /* Check for CBM_CREATDIB */
> if (Init & CBM_CREATDIB)
> {
>     /* CBM_CREATDIB needs Data. */
>    if (!Data)
>    {
>        return 0;
>    if (pbmiConverted == NULL)
>    {
>        DPRINT1("CBM_CREATDIB needs a BITMAINFO!\n");
>        goto Exit;
>    }
>
>    /* It only works with PAL or RGB */
>    if (ColorUse > DIB_PAL_COLORS)
>    {
>        DPRINT1("Invalid ColorUse: %lu\n", ColorUse);
>        GdiSetLastError(ERROR_INVALID_PARAMETER);
>        return 0;
>        goto Exit;
>    }
>
>    /* Use the header from the data */
>
> @@ -434,38 +441,48 @@
>      /* Header is required */
>      if (!Header)
>      {
>    DPRINT1("Header is NULL\n");
>    GdiSetLastError(ERROR_INVALID_PARAMETER);
>    return 0;
>    goto Exit;
>
> }
>
> /* Get the bitmap format and dimensions */
>    if (DIB_GetBitmapInfo(Header, &width, &height, &planes, &bpp, &compr, &dibsize) == -1)
>    {
>    DPRINT1("DIB_GetBitmapInfo failed!\n");
>    GdiSetLastError(ERROR_INVALID_PARAMETER);
>    return NULL;
>    goto Exit;
>
> }
>
> /* Check if the Compr is incompatible */
>    if ((compr == BI_JPEG) || (compr == BI_PNG) || (compr == BI_BITFIELDS))
>    return 0;
>
> {
>    DPRINT1("invalid compr: %lu!\n", compr);
>    goto Exit;
>
> }
>
> /* Only DIB_RGB_COLORS (0), DIB_PAL_COLORS (1) and 2 are valid. */
> if (ColorUse > DIB_PAL_COLORS + 1)
> {
>    DPRINT1("invalid compr: %lu!\n", compr);
>    GdiSetLastError(ERROR_INVALID_PARAMETER);
>    return 0;
>    goto Exit;
>
> }
>
> /* If some Bits are given, only DIB_PAL_COLORS and DIB_RGB_COLORS are valid */
>    if (Bits && (ColorUse > DIB_PAL_COLORS))
>    {
>    DPRINT1("Invalid ColorUse: %lu\n", ColorUse);
>    GdiSetLastError(ERROR_INVALID_PARAMETER);
>    return 0;
>    goto Exit;
>
> }
>
> /* Negative width is not allowed */
>    if (width < 0)
>    return 0;
>
> {
>    DPRINT1("Negative width: %li\n", width);
>    goto Exit;
>
> }
>
> /* Top-down DIBs have a negative height. */
> height = abs(height);
>
> @@ -473,13 +490,13 @@
>  // For Icm support.
>  // GdiGetHandleUserData(hdc, GDI_OBJECT_TYPE_DC, (PVOID)&pDc_Attr))
> if (Data)
> if (pbmiConverted)
> {
>     _SEH2_TRY
>     {
>        cjBmpScanSize = GdiGetBitmapBitsSize((BITMAPINFO *) Data);
>        CalculateColorTableSize(&Data->bmiHeader, &ColorUse, &InfoSize);
>        InfoSize += Data->bmiHeader.biSize;
>        cjBmpScanSize = GdiGetBitmapBitsSize(pbmiConverted);
>        CalculateColorTableSize(&pbmiConverted->bmiHeader, &ColorUse, &InfoSize);
>        InfoSize += pbmiConverted->bmiHeader.biSize;
>    }
>    _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
>    {
>
> @@ -490,8 +507,9 @@
>  if (!NT_SUCCESS(Status))
>  {
>    DPRINT1("Got an exception!\n");
>    GdiSetLastError(ERROR_INVALID_PARAMETER);
>    return NULL;
>    goto Exit;
>
> }
>
> DPRINT("pBMI %p, Size bpp %u, dibsize %d, Conv %u, BSS %u\n", Data, bpp, dibsize, InfoSize,
>
> @@ -501,9 +519,18 @@
>          hBmp = GetStockObject(DEFAULT_BITMAP);
>      else
>      {
>    hBmp = NtGdiCreateDIBitmapInternal(hDC, width, height, Init, (LPBYTE) Bits,
>        (LPBITMAPINFO) Data, ColorUse, InfoSize, cjBmpScanSize, 0, 0);
> }
>    hBmp = NtGdiCreateDIBitmapInternal(hDC, width, height, Init, (LPBYTE)Bits,
>        (LPBITMAPINFO)pbmiConverted, ColorUse, InfoSize, cjBmpScanSize, 0, 0);
> }
>
> +Exit:
> /* Cleanup converted BITMAPINFO */
> if ((pbmiConverted != NULL) && (pbmiConverted != Data))
> {
>    RtlFreeHeap(RtlGetProcessHeap(), 0, pbmiConverted);
> }
> return hBmp;
>
> }
>
> Modified: trunk/reactos/win32ss/gdi/ntgdi/dibobj.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/gdi/ntgdi/dibobj.c?...
> ==============================================================================
> --- trunk/reactos/win32ss/gdi/ntgdi/dibobj.c	[iso-8859-1] (original)
> +++ trunk/reactos/win32ss/gdi/ntgdi/dibobj.c	[iso-8859-1] Tue Dec 29 20:28:23 2015
> @@ -259,11 +259,37 @@
>      POINTL		ptSrc;
>      EXLATEOBJ	exlo;
>      PPALETTE    ppalDIB = 0;
>
> ULONG cjSizeImage;
>
> if (!bmi) return 0;
> if (bmi->bmiHeader.biSizeImage > cjMaxBits)
> {
> /* Check if the header provided an image size */
> if (bmi->bmiHeader.biSizeImage != 0)
> {
>    /* Use the given size */
>    cjSizeImage = bmi->bmiHeader.biSizeImage;
> }
> /* Otherwise check for uncompressed formats */
> else if ((bmi->bmiHeader.biCompression == BI_RGB) ||
>         (bmi->bmiHeader.biCompression == BI_BITFIELDS))
> {
>    /* Calculate the image size */
>    cjSizeImage = DIB_GetDIBImageBytes(bmi->bmiHeader.biWidth,
>                                       ScanLines,
>                                       bmi->bmiHeader.biBitCount);
> }
> else
> {
>    /* Compressed format without a size. This is invalid. */
>    DPRINT1("Compressed format without a size!");
>    return 0;
> }
> /* Check if the size that we have is ok */
> if (cjSizeImage > cjMaxBits)
> {
>    DPRINT1("Size too large! cjSizeImage = %lu, cjMaxBits = %lu\n",
>            cjSizeImage, cjMaxBits);
>    return 0;
> }
>
> @@ -273,7 +299,7 @@
>                                       BitmapFormat(bmi->bmiHeader.biBitCount,
>                                                    bmi->bmiHeader.biCompression),
>                                       bmi->bmiHeader.biHeight < 0 ? BMF_TOPDOWN : 0,
>                                 bmi->bmiHeader.biSizeImage,
>                                 cjSizeImage,
>                                 (PVOID)Bits,
>                                 0);
> if (!SourceBitmap)
>
> @@ -314,6 +340,8 @@
>      rcDst.right = psurfDst->SurfObj.sizlBitmap.cx;
>      ptSrc.x = 0;
>      ptSrc.y = 0;
>
> NT_ASSERT(psurfSrc->SurfObj.cjBits <= cjMaxBits);
>
> result = IntEngCopyBits(&psurfDst->SurfObj,
>                         &psurfSrc->SurfObj,
>
> @@ -1366,7 +1394,11 @@
>              /* Undocumented flag which creates a DDB of the format specified by the bitmap info. */
>              handle = IntCreateCompatibleBitmap(Dc, width, height, planes, bpp);
>              if (!handle)
>        {
>            DPRINT1("IntCreateCompatibleBitmap() failed!\n");
>            return NULL;
>        }
>        /* The palette must also match the given data */
>        Surface = SURFACE_ShareLockSurface(handle);
>        ASSERT(Surface);
>
> @@ -1430,6 +1462,7 @@
>          safeBits = ExAllocatePoolWithTag(PagedPool, cjMaxBits, TAG_DIB);
>          if(!safeBits)
>          {
>        DPRINT1("Failed to allocate %lu bytes\n", cjMaxBits);
>        EngSetLastError(ERROR_NOT_ENOUGH_MEMORY);
>        return NULL;
>    }
>
> @@ -1452,6 +1485,7 @@
>  if(!NT_SUCCESS(Status))
>  {
>    DPRINT1("Got an exception! pjInit = %p\n", pjInit);
>    SetLastNtError(Status);
>    goto cleanup;
> }
>
> @@ -1498,6 +1532,7 @@
>          hdcDest = NtGdiCreateCompatibleDC(0);
>          if(!hdcDest)
>          {
>        DPRINT1("NtGdiCreateCompatibleDC failed\n");
>        return NULL;
>    }
> }
>
> @@ -1509,6 +1544,7 @@
>      Dc = DC_LockDc(hdcDest);
>      if (!Dc)
>      {
>    DPRINT1("Failed to lock hdcDest %p\n", hdcDest);
>    EngSetLastError(ERROR_INVALID_HANDLE);
>    return NULL;
> }