Nikolay Sivov : wincodecs/icoformat: Improve input data validation on decoder initialization.
Alexandre Julliard
julliard at winehq.org
Tue Oct 9 16:22:47 CDT 2018
Module: wine
Branch: master
Commit: e72983b5124eba463f76bc5c5de1dae2485ef42e
URL: https://source.winehq.org/git/wine.git/?a=commit;h=e72983b5124eba463f76bc5c5de1dae2485ef42e
Author: Nikolay Sivov <nsivov at codeweavers.com>
Date: Tue Oct 9 09:41:49 2018 +0300
wincodecs/icoformat: Improve input data validation on decoder initialization.
Signed-off-by: Nikolay Sivov <nsivov at codeweavers.com>
Signed-off-by: Vincent Povirk <vincent at codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard at winehq.org>
---
dlls/windowscodecs/icoformat.c | 34 +++++-
dlls/windowscodecs/tests/icoformat.c | 207 ++++++++++++++++++++++++-----------
2 files changed, 173 insertions(+), 68 deletions(-)
diff --git a/dlls/windowscodecs/icoformat.c b/dlls/windowscodecs/icoformat.c
index 5e38ee0..dc08fd5 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 c53739d..38db51c 100644
--- a/dlls/windowscodecs/tests/icoformat.c
+++ b/dlls/windowscodecs/tests/icoformat.c
@@ -26,72 +26,114 @@
#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 */
+ {
+ 16, /* width */
+ 16, /* 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"
+
+#define test_ico_data(a, b, c) test_ico_data_(a, b, c, 0, __LINE__)
+#define test_ico_data_todo(a, b, c) test_ico_data_(a, b, c, 1, __LINE__)
+static void test_ico_data_(void *data, DWORD data_size, HRESULT init_hr, int todo, unsigned int line)
{
IWICBitmapDecoder *decoder;
IWICImagingFactory *factory;
@@ -108,8 +150,7 @@ static void test_bad_icondirentry_size(void)
ok(hr == S_OK, "CreateStream failed, hr=%x\n", hr);
if (SUCCEEDED(hr))
{
- hr = IWICStream_InitializeFromMemory(icostream, testico_bad_icondirentry_size,
- sizeof(testico_bad_icondirentry_size));
+ hr = IWICStream_InitializeFromMemory(icostream, data, data_size);
ok(hr == S_OK, "InitializeFromMemory failed, hr=%x\n", hr);
if (SUCCEEDED(hr))
@@ -123,7 +164,8 @@ static void test_bad_icondirentry_size(void)
{
hr = IWICBitmapDecoder_Initialize(decoder, (IStream*)icostream,
WICDecodeMetadataCacheOnDemand);
- ok(hr == S_OK, "Initialize failed, hr=%x\n", hr);
+ todo_wine_if(todo)
+ ok_(__FILE__, line)(hr == init_hr, "Initialize failed, hr=%x\n", hr);
if (SUCCEEDED(hr))
{
@@ -163,11 +205,44 @@ static void test_bad_icondirentry_size(void)
IWICImagingFactory_Release(factory);
}
+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), S_OK);
+
+ /* Invalid DIRENTRY data size/offset. */
+ ico = ico_1;
+ ico.direntry.dwDIBOffset = sizeof(ico);
+ test_ico_data(&ico, sizeof(ico), WINCODEC_ERR_BADIMAGE);
+
+ ico = ico_1;
+ ico.direntry.dwDIBSize = sizeof(ico);
+ test_ico_data(&ico, sizeof(ico), WINCODEC_ERR_BADIMAGE);
+
+ /* Header fields validation. */
+ ico = ico_1;
+ ico.header.idReserved = 1;
+ test_ico_data_todo(&ico, sizeof(ico), S_OK);
+ ico.header.idReserved = 0;
+ ico.header.idType = 100;
+ test_ico_data_todo(&ico, sizeof(ico), S_OK);
+
+ /* Premature end of data. */
+ ico = ico_1;
+ test_ico_data(&ico, sizeof(ico.header) - 1, WINCODEC_ERR_STREAMREAD);
+ test_ico_data(&ico, sizeof(ico.header) + sizeof(ico.direntry) - 1, WINCODEC_ERR_BADIMAGE);
+}
+
START_TEST(icoformat)
{
CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
- test_bad_icondirentry_size();
+ test_decoder();
CoUninitialize();
}
More information about the wine-cvs
mailing list