[PATCH] wincodecs/icoformat: Improve input data validation on decoder initialization.
Nikolay Sivov
nsivov at codeweavers.com
Mon Oct 8 00:05:53 CDT 2018
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45942
Signed-off-by: Nikolay Sivov <nsivov at codeweavers.com>
---
dlls/windowscodecs/icoformat.c | 34 ++-
dlls/windowscodecs/tests/icoformat.c | 324 +++++++++++++++++----------
2 files changed, 232 insertions(+), 126 deletions(-)
diff --git a/dlls/windowscodecs/icoformat.c b/dlls/windowscodecs/icoformat.c
index 5e38ee0d0f..dc08fd5576 100644
--- a/dlls/windowscodecs/icoformat.c
+++ b/dlls/windowscodecs/icoformat.c
@@ -511,6 +511,9 @@ static HRESULT WINAPI IcoDecoder_Initialize(IWICBitmapDecoder *iface, IStream *p
LARGE_INTEGER seek;
HRESULT hr;
ULONG bytesread;
+ STATSTG statstg;
+ unsigned int i;
+
TRACE("(%p,%p,%x)\n", iface, pIStream, cacheOptions);
EnterCriticalSection(&This->lock);
@@ -527,14 +530,41 @@ static HRESULT WINAPI IcoDecoder_Initialize(IWICBitmapDecoder *iface, IStream *p
hr = IStream_Read(pIStream, &This->header, sizeof(ICONHEADER), &bytesread);
if (FAILED(hr)) goto end;
- if (bytesread != sizeof(ICONHEADER) ||
- This->header.idReserved != 0 ||
+
+ if (bytesread != sizeof(ICONHEADER))
+ {
+ hr = WINCODEC_ERR_STREAMREAD;
+ goto end;
+ }
+
+ if (This->header.idReserved != 0 ||
This->header.idType != 1)
{
hr = E_FAIL;
goto end;
}
+ hr = IStream_Stat(pIStream, &statstg, STATFLAG_NONAME);
+ if (FAILED(hr))
+ {
+ WARN("Stat() failed, hr %#x.\n", hr);
+ goto end;
+ }
+
+ for (i = 0; i < This->header.idCount; i++)
+ {
+ ICONDIRENTRY direntry;
+
+ hr = IStream_Read(pIStream, &direntry, sizeof(direntry), &bytesread);
+ if (FAILED(hr)) goto end;
+
+ if (bytesread != sizeof(direntry) || (direntry.dwDIBSize + direntry.dwDIBOffset > statstg.cbSize.QuadPart))
+ {
+ hr = WINCODEC_ERR_BADIMAGE;
+ goto end;
+ }
+ }
+
This->initialized = TRUE;
This->stream = pIStream;
IStream_AddRef(pIStream);
diff --git a/dlls/windowscodecs/tests/icoformat.c b/dlls/windowscodecs/tests/icoformat.c
index c53739dbd6..9324b59a6c 100644
--- a/dlls/windowscodecs/tests/icoformat.c
+++ b/dlls/windowscodecs/tests/icoformat.c
@@ -26,148 +26,224 @@
#include "wincodec.h"
#include "wine/test.h"
-static unsigned char testico_bad_icondirentry_size[] = {
- /* ICONDIR */
- 0, 0, /* reserved */
- 1, 0, /* type */
- 1, 0, /* count */
+#include "pshpack1.h"
+
+struct ICONHEADER
+{
+ WORD idReserved;
+ WORD idType;
+ WORD idCount;
+};
+
+struct ICONDIRENTRY
+{
+ BYTE bWidth;
+ BYTE bHeight;
+ BYTE bColorCount;
+ BYTE bReserved;
+ WORD wPlanes;
+ WORD wBitCount;
+ DWORD dwDIBSize;
+ DWORD dwDIBOffset;
+};
+
+struct test_ico
+{
+ struct ICONHEADER header;
+ struct ICONDIRENTRY direntry;
+ BITMAPINFOHEADER bmi;
+ unsigned char data[512];
+};
+
+static const struct test_ico ico_1 =
+{
+ /* ICONHEADER */
+ {
+ 0, /* reserved */
+ 1, /* type */
+ 1, /* count */
+ },
/* ICONDIRENTRY */
- 2, /* width */
- 2, /* height */
- 2, /* colorCount */
- 0, /* reserved */
- 1,0, /* planes */
- 8,0, /* bitCount */
- (40+2*4+16*16+16*4) & 0xFF,((40+2*4+16*16+16*4) >> 8) & 0xFF,0,0, /* bytesInRes */
- 22,0,0,0, /* imageOffset */
+ {
+ 2, /* width */
+ 2, /* height */
+ 2, /* color count */
+ 0, /* reserved */
+ 1, /* planes */
+ 8, /* bitcount*/
+ 40 + 2*4 + 16 * 16 + 16 * 4, /* data size */
+ 22 /* data offset */
+ },
/* BITMAPINFOHEADER */
- 40,0,0,0, /* header size */
- 16,0,0,0, /* width */
- 2*16,0,0,0, /* height (XOR+AND rows) */
- 1,0, /* planes */
- 8,0, /* bit count */
- 0,0,0,0, /* compression */
- 0,0,0,0, /* sizeImage */
- 0,0,0,0, /* x pels per meter */
- 0,0,0,0, /* y pels per meter */
- 2,0,0,0, /* clrUsed */
- 0,0,0,0, /* clrImportant */
- /* palette */
- 0,0,0,0,
- 0xFF,0xFF,0xFF,0,
- /* XOR mask */
- 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
- 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
- 0,1,0,0,0,0,0,0,0,0,0,0,0,1,0,0,
- 0,1,0,0,0,0,0,0,0,0,0,0,0,1,0,0,
- 0,1,0,0,0,0,0,0,0,0,0,0,0,1,0,0,
- 0,0,1,0,0,0,0,0,0,0,0,0,1,0,0,0,
- 0,0,1,0,0,0,0,0,0,0,0,0,1,0,0,0,
- 0,0,1,0,0,0,0,0,0,0,0,0,1,0,0,0,
- 0,0,0,1,0,0,0,0,0,0,0,1,0,0,0,0,
- 0,0,0,1,0,0,0,0,0,0,0,1,0,0,0,0,
- 0,0,0,1,0,0,0,1,0,0,0,1,0,0,0,0,
- 0,0,0,0,1,0,1,0,1,0,1,0,0,0,0,0,
- 0,0,0,0,0,1,0,0,0,1,0,0,0,0,0,0,
- 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
- 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
- 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
- /* AND mask */
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0,
- 0,0,0,0
+ {
+ sizeof(BITMAPINFOHEADER), /* header size */
+ 16, /* width */
+ 2*16, /* height (XOR+AND rows) */
+ 1, /* planes */
+ 8, /* bit count */
+ 0, /* compression */
+ 0, /* sizeImage */
+ 0, /* x pels per meter */
+ 0, /* y pels per meter */
+ 2, /* clrUsed */
+ 0, /* clrImportant */
+ },
+ {
+ /* palette */
+ 0,0,0,0,
+ 0xFF,0xFF,0xFF,0,
+ /* XOR mask */
+ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+ 0,1,0,0,0,0,0,0,0,0,0,0,0,1,0,0,
+ 0,1,0,0,0,0,0,0,0,0,0,0,0,1,0,0,
+ 0,1,0,0,0,0,0,0,0,0,0,0,0,1,0,0,
+ 0,0,1,0,0,0,0,0,0,0,0,0,1,0,0,0,
+ 0,0,1,0,0,0,0,0,0,0,0,0,1,0,0,0,
+ 0,0,1,0,0,0,0,0,0,0,0,0,1,0,0,0,
+ 0,0,0,1,0,0,0,0,0,0,0,1,0,0,0,0,
+ 0,0,0,1,0,0,0,0,0,0,0,1,0,0,0,0,
+ 0,0,0,1,0,0,0,1,0,0,0,1,0,0,0,0,
+ 0,0,0,0,1,0,1,0,1,0,1,0,0,0,0,0,
+ 0,0,0,0,0,1,0,0,0,1,0,0,0,0,0,0,
+ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+ /* AND mask */
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0,
+ 0,0,0,0
+ }
};
-static void test_bad_icondirentry_size(void)
+#include "poppack.h"
+
+static IWICStream *create_stream(void)
{
- IWICBitmapDecoder *decoder;
IWICImagingFactory *factory;
+ IWICStream *stream;
HRESULT hr;
- IWICStream *icostream;
- IWICBitmapFrameDecode *framedecode = NULL;
- hr = CoCreateInstance(&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER,
- &IID_IWICImagingFactory, (void**)&factory);
- ok(hr == S_OK, "CoCreateInstance failed, hr=%x\n", hr);
- if (FAILED(hr)) return;
+ hr = CoCreateInstance(&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER, &IID_IWICImagingFactory, (void **)&factory);
+ ok(hr == S_OK, "Failed to create imaging factory, hr %#x.\n", hr);
+ if (FAILED(hr))
+ return NULL;
+
+ hr = IWICImagingFactory_CreateStream(factory, &stream);
+ ok(hr == S_OK, "Failed to create a stream, hr %#x.\n", hr);
+
+ IWICImagingFactory_Release(factory);
- hr = IWICImagingFactory_CreateStream(factory, &icostream);
- ok(hr == S_OK, "CreateStream failed, hr=%x\n", hr);
- if (SUCCEEDED(hr))
+ return stream;
+}
+
+#define test_ico_data(a, b, c, d, e) test_ico_data_(a, b, c, d, e, 0, __LINE__)
+#define test_ico_data_todo(a, b, c, d, e) test_ico_data_(a, b, c, d, e, 1, __LINE__)
+static void test_ico_data_(void *data, DWORD data_size, UINT expected_width, UINT expected_height, HRESULT init, int todo, unsigned int line)
+{
+ IWICBitmapFrameDecode *framedecode;
+ IWICBitmapSource *thumbnail;
+ IWICBitmapDecoder *decoder;
+ UINT width, height;
+ IWICStream *stream;
+ HRESULT hr;
+
+ stream = create_stream();
+
+ hr = IWICStream_InitializeFromMemory(stream, data, data_size);
+ ok(hr == S_OK, "InitializeFromMemory failed, hr %#x.\n", hr);
+
+ hr = CoCreateInstance(&CLSID_WICIcoDecoder, NULL, CLSCTX_INPROC_SERVER,
+ &IID_IWICBitmapDecoder, (void **)&decoder);
+ ok(hr == S_OK, "Failed to create ICO decoder, hr %#x.\n", hr);
+
+ hr = IWICBitmapDecoder_Initialize(decoder, (IStream *)stream, WICDecodeMetadataCacheOnDemand);
+todo_wine_if(todo)
+ ok_(__FILE__, line)(hr == init, "Unexpected return value hr %#x.\n", hr);
+
+ if (FAILED(hr))
{
- hr = IWICStream_InitializeFromMemory(icostream, testico_bad_icondirentry_size,
- sizeof(testico_bad_icondirentry_size));
- ok(hr == S_OK, "InitializeFromMemory failed, hr=%x\n", hr);
-
- if (SUCCEEDED(hr))
- {
- hr = CoCreateInstance(&CLSID_WICIcoDecoder, NULL, CLSCTX_INPROC_SERVER,
- &IID_IWICBitmapDecoder, (void**)&decoder);
- ok(hr == S_OK, "CoCreateInstance failed, hr=%x\n", hr);
- }
-
- if (SUCCEEDED(hr))
- {
- hr = IWICBitmapDecoder_Initialize(decoder, (IStream*)icostream,
- WICDecodeMetadataCacheOnDemand);
- ok(hr == S_OK, "Initialize failed, hr=%x\n", hr);
-
- if (SUCCEEDED(hr))
- {
- hr = IWICBitmapDecoder_GetFrame(decoder, 0, &framedecode);
- ok(hr == S_OK, "GetFrame failed, hr=%x\n", hr);
- }
-
- if (SUCCEEDED(hr))
- {
- UINT width, height;
- IWICBitmapSource *thumbnail;
-
- width = height = 0;
- hr = IWICBitmapFrameDecode_GetSize(framedecode, &width, &height);
- ok(hr == S_OK, "GetFrameSize failed, hr=%x\n", hr);
- ok(width == 16 && height == 16, "framesize=%ux%u\n", width, height);
-
- hr = IWICBitmapFrameDecode_GetThumbnail(framedecode, &thumbnail);
- ok(hr == S_OK, "GetThumbnail failed, hr=%x\n", hr);
- if (hr == S_OK)
- {
- width = height = 0;
- hr = IWICBitmapSource_GetSize(thumbnail, &width, &height);
- ok(hr == S_OK, "GetFrameSize failed, hr=%x\n", hr);
- ok(width == 16 && height == 16, "framesize=%ux%u\n", width, height);
- IWICBitmapSource_Release(thumbnail);
- }
- IWICBitmapFrameDecode_Release(framedecode);
- }
-
- IWICBitmapDecoder_Release(decoder);
- }
-
- IWICStream_Release(icostream);
+ IWICStream_Release(stream);
+ IWICBitmapDecoder_Release(decoder);
+ return;
}
- IWICImagingFactory_Release(factory);
+ hr = IWICBitmapDecoder_GetFrame(decoder, 0, &framedecode);
+ ok(hr == S_OK, "GetFrame failed, hr=%x\n", hr);
+
+ width = height = 0;
+ hr = IWICBitmapFrameDecode_GetSize(framedecode, &width, &height);
+ ok(hr == S_OK, "Failed to get frame size, hr %#x.\n", hr);
+ ok(width == expected_width && height == expected_height, "Unexpected frame size %ux%u.\n", width, height);
+
+ hr = IWICBitmapFrameDecode_GetThumbnail(framedecode, &thumbnail);
+ ok(hr == S_OK, "GetThumbnail failed, hr=%x\n", hr);
+ if (hr == S_OK)
+ {
+ width = height = 0;
+ hr = IWICBitmapSource_GetSize(thumbnail, &width, &height);
+ ok(hr == S_OK, "Failed to get thumbnail size, hr %#x.\n", hr);
+ ok(width == expected_width && height == expected_height, "Unexpected thumbnail size %ux%u.\n", width, height);
+ IWICBitmapSource_Release(thumbnail);
+ }
+
+ IWICBitmapFrameDecode_Release(framedecode);
+
+ IWICBitmapDecoder_Release(decoder);
+ IWICStream_Release(stream);
+}
+
+static void test_decoder(void)
+{
+ struct test_ico ico;
+
+ /* Icon size specified in ICONDIRENTRY does not match bitmap header. */
+ ico = ico_1;
+ ico.direntry.bWidth = 2;
+ ico.direntry.bHeight = 2;
+ test_ico_data(&ico, sizeof(ico), 16, 16, S_OK);
+
+ /* Invalid DIRENTRY data size/offset. */
+ ico = ico_1;
+ ico.direntry.dwDIBOffset = sizeof(ico);
+ test_ico_data(&ico, sizeof(ico), 0, 0, WINCODEC_ERR_BADIMAGE);
+
+ ico = ico_1;
+ ico.direntry.dwDIBSize = sizeof(ico);
+ test_ico_data(&ico, sizeof(ico), 0, 0, WINCODEC_ERR_BADIMAGE);
+
+ /* Header fields validation. */
+ ico = ico_1;
+ ico.header.idReserved = 1;
+ test_ico_data_todo(&ico, sizeof(ico), 16, 16, S_OK);
+ ico.header.idReserved = 0;
+ ico.header.idType = 100;
+ test_ico_data_todo(&ico, sizeof(ico), 16, 16, S_OK);
+
+ /* Premature end of data. */
+ ico = ico_1;
+ test_ico_data(&ico, sizeof(ico.header) - 1, 0, 0, WINCODEC_ERR_STREAMREAD);
+ test_ico_data(&ico, sizeof(ico.header) + sizeof(ico.direntry) - 1, 0, 0, WINCODEC_ERR_BADIMAGE);
}
START_TEST(icoformat)
{
CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
- test_bad_icondirentry_size();
+ test_decoder();
CoUninitialize();
}
--
2.19.0
More information about the wine-devel
mailing list