windowscodecs: Fix *_CopyPixels functions to properly handle a NULL rectangle

Krzysztof Nowicki krissn at op.pl
Sat Oct 16 15:40:55 CDT 2010


According to the documentation found on MSDN in case of the 
IWICBitmapSource::CopyPixels function, passing a NULL rectangle pointer means
that the whole bitmap should be taken into account.

The existing code incorrectly assumed that the rectangle pointer will never be
NULL, which could lead to NULL pointer dereferences.

This patch fixes bug #22957.

---
 dlls/windowscodecs/converter.c  |   16 +++++++++++
 dlls/windowscodecs/fliprotate.c |   22 +++++++++++++--
 dlls/windowscodecs/gifformat.c  |   26 +++++++++++++-----
 dlls/windowscodecs/jpegformat.c |   18 +++++++++++-
 dlls/windowscodecs/main.c       |   33 +++++++++++++++++-----
 dlls/windowscodecs/tiffformat.c |   58 +++++++++++++++++++++++---------------
 6 files changed, 132 insertions(+), 41 deletions(-)

diff --git a/dlls/windowscodecs/converter.c b/dlls/windowscodecs/converter.c
index 86cdf55..a83f4a5 100644
--- a/dlls/windowscodecs/converter.c
+++ b/dlls/windowscodecs/converter.c
@@ -850,11 +850,27 @@ static HRESULT WINAPI FormatConverter_CopyPixels(IWICFormatConverter *iface,
     const WICRect *prc, UINT cbStride, UINT cbBufferSize, BYTE *pbBuffer)
 {
     FormatConverter *This = (FormatConverter*)iface;
+    WICRect rc;
+    HRESULT hr;
     TRACE("(%p,%p,%u,%u,%p)\n", iface, prc, cbStride, cbBufferSize, pbBuffer);
 
     if (This->source)
+    {
+        if (!prc)
+        {
+            UINT width, height;
+            hr = IWICBitmapSource_GetSize(This->source, &width, &height);
+            if (FAILED(hr)) return hr;
+            rc.X = 0;
+            rc.Y = 0;
+            rc.Width = width;
+            rc.Height = height;
+            prc = &rc;
+        }
+
         return This->dst_format->copy_function(This, prc, cbStride, cbBufferSize,
             pbBuffer, This->src_format->format);
+    }
     else
         return WINCODEC_ERR_NOTINITIALIZED;
 }
diff --git a/dlls/windowscodecs/fliprotate.c b/dlls/windowscodecs/fliprotate.c
index 7dac252..9c3d1e6 100644
--- a/dlls/windowscodecs/fliprotate.c
+++ b/dlls/windowscodecs/fliprotate.c
@@ -140,6 +140,7 @@ static HRESULT WINAPI FlipRotator_CopyPixels(IWICBitmapFlipRotator *iface,
     UINT y;
     UINT srcy, srcwidth, srcheight;
     WICRect rc;
+    WICRect rect;
 
     TRACE("(%p,%p,%u,%u,%p)\n", iface, prc, cbStride, cbBufferSize, pbBuffer);
 
@@ -155,16 +156,31 @@ static HRESULT WINAPI FlipRotator_CopyPixels(IWICBitmapFlipRotator *iface,
     hr = IWICBitmapSource_GetSize(This->source, &srcwidth, &srcheight);
     if (FAILED(hr)) return hr;
 
-    for (y=prc->Y; y - prc->Y < prc->Height; y++)
+    if (prc)
+    {
+        memcpy(&rect, prc, sizeof(rect));
+    }
+    else
+    {
+        UINT width, height;
+        hr = IWICBitmapSource_GetSize(iface, &width, &height);
+        if (FAILED(hr)) return hr;
+        rect.X = 0;
+        rect.Y = 0;
+        rect.Width = width;
+        rect.Height = height;
+    }
+
+    for (y=rect.Y; y - rect.Y < rect.Height; y++)
     {
         if (This->flip_y)
             srcy = srcheight - 1 - y;
         else
             srcy = y;
 
-        rc.X = prc->X;
+        rc.X = rect.X;
         rc.Y = srcy;
-        rc.Width = prc->Width;
+        rc.Width = rect.Width;
         rc.Height = 1;
 
         hr = IWICBitmapSource_CopyPixels(This->source, &rc, cbStride, cbStride,
diff --git a/dlls/windowscodecs/gifformat.c b/dlls/windowscodecs/gifformat.c
index 239d25d..1d8833c 100644
--- a/dlls/windowscodecs/gifformat.c
+++ b/dlls/windowscodecs/gifformat.c
@@ -184,20 +184,32 @@ static HRESULT copy_interlaced_pixels(const BYTE *srcbuffer,
     const BYTE *src;
     BYTE *dst;
     UINT y;
+    WICRect rect;
 
-    if (rc->X < 0 || rc->Y < 0 || rc->X+rc->Width > srcwidth || rc->Y+rc->Height > srcheight)
-        return E_INVALIDARG;
+    if (rc)
+    {
+        if (rc->X < 0 || rc->Y < 0 || rc->X+rc->Width > srcwidth || rc->Y+rc->Height > srcheight)
+            return E_INVALIDARG;
+        memcpy(&rect, rc, sizeof(rect));
+    }
+    else
+    {
+        rect.X = 0;
+        rect.Y = 0;
+        rect.Width = srcwidth;
+        rect.Height = srcheight;
+    }
 
-    if (dststride < rc->Width)
+    if (dststride < rect.Width)
         return E_INVALIDARG;
 
-    if ((dststride * rc->Height) > dstbuffersize)
+    if ((dststride * rect.Height) > dstbuffersize)
         return E_INVALIDARG;
 
-    row_offset = rc->X;
+    row_offset = rect.X;
 
     dst = dstbuffer;
-    for (y=rc->Y; y-rc->Y < rc->Height; y++)
+    for (y=rect.Y; y-rect.Y < rect.Height; y++)
     {
         if (y%8 == 0)
             src = srcbuffer + srcstride * (y/8);
@@ -208,7 +220,7 @@ static HRESULT copy_interlaced_pixels(const BYTE *srcbuffer,
         else /* y%2 == 1 */
             src = srcbuffer + srcstride * ((srcheight+1)/2 + y/2);
         src += row_offset;
-        memcpy(dst, src, rc->Width);
+        memcpy(dst, src, rect.Width);
         dst += dststride;
     }
     return S_OK;
diff --git a/dlls/windowscodecs/jpegformat.c b/dlls/windowscodecs/jpegformat.c
index 5243d94..e72e12a 100644
--- a/dlls/windowscodecs/jpegformat.c
+++ b/dlls/windowscodecs/jpegformat.c
@@ -519,8 +519,24 @@ static HRESULT WINAPI JpegDecoder_Frame_CopyPixels(IWICBitmapFrameDecode *iface,
     UINT data_size;
     UINT max_row_needed;
     jmp_buf jmpbuf;
+    WICRect rect;
     TRACE("(%p,%p,%u,%u,%p)\n", iface, prc, cbStride, cbBufferSize, pbBuffer);
 
+    if (prc)
+    {
+        if (prc->X < 0 || prc->Y < 0 || prc->X+prc->Width > This->cinfo.output_width ||
+            prc->Y+prc->Height > This->cinfo.output_height)
+            return E_INVALIDARG;
+        memcpy(&rect, prc, sizeof(rect));
+    }
+    else
+    {
+        rect.X = 0;
+        rect.Y = 0;;
+        rect.Width = This->cinfo.output_width;
+        rect.Height = This->cinfo.output_height;
+    }
+
     if (This->cinfo.out_color_space == JCS_GRAYSCALE) bpp = 8;
     else if (This->cinfo.out_color_space == JCS_CMYK) bpp = 32;
     else bpp = 24;
@@ -528,7 +544,7 @@ static HRESULT WINAPI JpegDecoder_Frame_CopyPixels(IWICBitmapFrameDecode *iface,
     stride = bpp * This->cinfo.output_width;
     data_size = stride * This->cinfo.output_height;
 
-    max_row_needed = prc->Y + prc->Height;
+    max_row_needed = rect.Y + rect.Height;
     if (max_row_needed > This->cinfo.output_height) return E_INVALIDARG;
 
     EnterCriticalSection(&This->lock);
diff --git a/dlls/windowscodecs/main.c b/dlls/windowscodecs/main.c
index ff543d5..f4c8593 100644
--- a/dlls/windowscodecs/main.c
+++ b/dlls/windowscodecs/main.c
@@ -53,19 +53,38 @@ HRESULT copy_pixels(UINT bpp, const BYTE *srcbuffer,
 {
     UINT bytesperrow;
     UINT row_offset; /* number of bits into the source rows where the data starts */
+    WICRect rect;
 
-    if (rc->X < 0 || rc->Y < 0 || rc->X+rc->Width > srcwidth || rc->Y+rc->Height > srcheight)
-        return E_INVALIDARG;
+    if (rc)
+    {
+        if (rc->X < 0 || rc->Y < 0 || rc->X+rc->Width > srcwidth || rc->Y+rc->Height > srcheight)
+            return E_INVALIDARG;
+        memcpy(&rect, rc, sizeof(rect));
+    }
+    else
+    {
+        rect.X = 0;
+        rect.Y = 0;
+        rect.Width = srcwidth;
+        rect.Height = srcheight;
+    }
 
-    bytesperrow = ((bpp * rc->Width)+7)/8;
+    bytesperrow = ((bpp * rect.Width)+7)/8;
 
     if (dststride < bytesperrow)
         return E_INVALIDARG;
 
-    if ((dststride * rc->Height) > dstbuffersize)
+    if ((dststride * rect.Height) > dstbuffersize)
         return E_INVALIDARG;
 
-    row_offset = rc->X * bpp;
+    /* if the whole bitmap is copied and the buffer format matches then it's a matter of a single memcpy */
+    if (!rc && srcstride == dststride)
+    {
+        memcpy(dstbuffer, srcbuffer, srcstride * srcheight);
+        return S_OK;
+    }
+
+    row_offset = rect.X * bpp;
 
     if (row_offset % 8 == 0)
     {
@@ -74,9 +93,9 @@ HRESULT copy_pixels(UINT bpp, const BYTE *srcbuffer,
         const BYTE *src;
         BYTE *dst;
 
-        src = srcbuffer + (row_offset / 8) + srcstride * rc->Y;
+        src = srcbuffer + (row_offset / 8) + srcstride * rect.Y;
         dst = dstbuffer;
-        for (row=0; row < rc->Height; row++)
+        for (row=0; row < rect.Height; row++)
         {
             memcpy(dst, src, bytesperrow);
             src += srcstride;
diff --git a/dlls/windowscodecs/tiffformat.c b/dlls/windowscodecs/tiffformat.c
index 645cfb0..c84f657 100644
--- a/dlls/windowscodecs/tiffformat.c
+++ b/dlls/windowscodecs/tiffformat.c
@@ -799,25 +799,37 @@ static HRESULT WINAPI TiffFrameDecode_CopyPixels(IWICBitmapFrameDecode *iface,
     HRESULT hr=S_OK;
     BYTE *dst_tilepos;
     UINT bytesperrow;
+    WICRect rect;
 
     TRACE("(%p,%p,%u,%u,%p)\n", iface, prc, cbStride, cbBufferSize, pbBuffer);
 
-    if (prc->X < 0 || prc->Y < 0 || prc->X+prc->Width > This->decode_info.width ||
-        prc->Y+prc->Height > This->decode_info.height)
-        return E_INVALIDARG;
+    if (prc)
+    {
+        if (prc->X < 0 || prc->Y < 0 || prc->X+prc->Width > This->decode_info.width ||
+            prc->Y+prc->Height > This->decode_info.height)
+            return E_INVALIDARG;
+        memcpy(&rect, prc, sizeof(rect));
+    }
+    else
+    {
+        rect.X = 0;
+        rect.Y = 0;
+        rect.Width = This->decode_info.width;
+        rect.Height = This->decode_info.height;
+    }
 
-    bytesperrow = ((This->decode_info.bpp * prc->Width)+7)/8;
+    bytesperrow = ((This->decode_info.bpp * rect.Width)+7)/8;
 
     if (cbStride < bytesperrow)
         return E_INVALIDARG;
 
-    if ((cbStride * prc->Height) > cbBufferSize)
+    if ((cbStride * rect.Height) > cbBufferSize)
         return E_INVALIDARG;
 
-    min_tile_x = prc->X / This->decode_info.tile_width;
-    min_tile_y = prc->Y / This->decode_info.tile_height;
-    max_tile_x = (prc->X+prc->Width-1) / This->decode_info.tile_width;
-    max_tile_y = (prc->Y+prc->Height-1) / This->decode_info.tile_height;
+    min_tile_x = rect.X / This->decode_info.tile_width;
+    min_tile_y = rect.Y / This->decode_info.tile_height;
+    max_tile_x = (rect.X+rect.Width-1) / This->decode_info.tile_width;
+    max_tile_y = (rect.Y+rect.Height-1) / This->decode_info.tile_height;
 
     EnterCriticalSection(&This->parent->lock);
 
@@ -832,32 +844,32 @@ static HRESULT WINAPI TiffFrameDecode_CopyPixels(IWICBitmapFrameDecode *iface,
 
             if (SUCCEEDED(hr))
             {
-                if (prc->X < tile_x * This->decode_info.tile_width)
+                if (rect.X < tile_x * This->decode_info.tile_width)
                     rc.X = 0;
                 else
-                    rc.X = prc->X - tile_x * This->decode_info.tile_width;
+                    rc.X = rect.X - tile_x * This->decode_info.tile_width;
 
-                if (prc->Y < tile_y * This->decode_info.tile_height)
+                if (rect.Y < tile_y * This->decode_info.tile_height)
                     rc.Y = 0;
                 else
-                    rc.Y = prc->Y - tile_y * This->decode_info.tile_height;
+                    rc.Y = rect.Y - tile_y * This->decode_info.tile_height;
 
-                if (prc->X+prc->Width > (tile_x+1) * This->decode_info.tile_width)
+                if (rect.X+rect.Width > (tile_x+1) * This->decode_info.tile_width)
                     rc.Width = This->decode_info.tile_width - rc.X;
-                else if (prc->X < tile_x * This->decode_info.tile_width)
-                    rc.Width = prc->Width + prc->X - tile_x * This->decode_info.tile_width;
+                else if (rect.X < tile_x * This->decode_info.tile_width)
+                    rc.Width = rect.Width + rect.X - tile_x * This->decode_info.tile_width;
                 else
-                    rc.Width = prc->Width;
+                    rc.Width = rect.Width;
 
-                if (prc->Y+prc->Height > (tile_y+1) * This->decode_info.tile_height)
+                if (rect.Y+rect.Height > (tile_y+1) * This->decode_info.tile_height)
                     rc.Height = This->decode_info.tile_height - rc.Y;
-                else if (prc->Y < tile_y * This->decode_info.tile_height)
-                    rc.Height = prc->Height + prc->Y - tile_y * This->decode_info.tile_height;
+                else if (rect.Y < tile_y * This->decode_info.tile_height)
+                    rc.Height = rect.Height + rect.Y - tile_y * This->decode_info.tile_height;
                 else
-                    rc.Height = prc->Height;
+                    rc.Height = rect.Height;
 
-                dst_tilepos = pbBuffer + (cbStride * ((rc.Y + tile_y * This->decode_info.tile_height) - prc->Y)) +
-                    ((This->decode_info.bpp * ((rc.X + tile_x * This->decode_info.tile_width) - prc->X) + 7) / 8);
+                dst_tilepos = pbBuffer + (cbStride * ((rc.Y + tile_y * This->decode_info.tile_height) - rect.Y)) +
+                    ((This->decode_info.bpp * ((rc.X + tile_x * This->decode_info.tile_width) - rect.X) + 7) / 8);
 
                 hr = copy_pixels(This->decode_info.bpp, This->cached_tile,
                     This->decode_info.tile_width, This->decode_info.tile_height, This->decode_info.tile_stride,
-- 
1.7.1




More information about the wine-patches mailing list