Fix crash in property parser for invalid image types

Author: chrdavis

PIFShellExtensionsLib/PIFParser.cpp

@@ -24,7 +24,11 @@ CBaseImageParser::CBaseImageParser() :
 
 CBaseImageParser::~CBaseImageParser()
 {
-    delete [] m_rgData;
+    if (m_rgData)
+    {
+        delete [] m_rgData;
+    }
+
     GdiplusShutdown(_gdiPlusToken);
 }
 

PIFShellExtensionsLib/PIFShellExtensions.cpp

@@ -456,53 +456,52 @@ HRESULT CPIFShellExt::_InitializeProperties()
         PROPVARIANT pv = {};
         CBaseImageParser* pImageParser = nullptr;
         hr = CBaseImageParser::CreateParserFromStream(_spStream, &pImageParser);
-
         if (SUCCEEDED(hr))
         {
             pv.vt = VT_LPWSTR;
             pv.pwszVal = L"image/bmp";
             hr = _spPropertyStore->SetValue(PKEY_MIMEType, pv);
-        }
 
-        if (SUCCEEDED(hr))
-        {
-            pv.vt = VT_UI4;
-            pv.ulVal = pImageParser->GetWidth();
-            hr = _spPropertyStore->SetValue(PKEY_Image_HorizontalSize, pv);
-        }
+            if (SUCCEEDED(hr))
+            {
+                pv.vt = VT_UI4;
+                pv.ulVal = pImageParser->GetWidth();
+                hr = _spPropertyStore->SetValue(PKEY_Image_HorizontalSize, pv);
+            }
 
-        if (SUCCEEDED(hr))
-        {
-            pv.vt = VT_UI4;
-            pv.ulVal = pImageParser->GetHeight();
-            hr = _spPropertyStore->SetValue(PKEY_Image_VerticalSize, pv);
-        }
+            if (SUCCEEDED(hr))
+            {
+                pv.vt = VT_UI4;
+                pv.ulVal = pImageParser->GetHeight();
+                hr = _spPropertyStore->SetValue(PKEY_Image_VerticalSize, pv);
+            }
 
-        if (SUCCEEDED(hr))
-        {
-            pv.vt = VT_UI4;
-            pv.ulVal = pImageParser->GetBitDepth();
-            hr = _spPropertyStore->SetValue(PKEY_Image_BitDepth, pv);
-        }
+            if (SUCCEEDED(hr))
+            {
+                pv.vt = VT_UI4;
+                pv.ulVal = pImageParser->GetBitDepth();
+                hr = _spPropertyStore->SetValue(PKEY_Image_BitDepth, pv);
+            }
 
-        if (SUCCEEDED(hr))
-        {
-            wchar_t szResolution[30];
-            hr = StringCchPrintf(szResolution, ARRAYSIZE(szResolution), L"%i x %i", pImageParser->GetWidth(), pImageParser->GetHeight());
-            pv.vt = VT_LPWSTR;
-            pv.pwszVal = szResolution;
-            hr = _spPropertyStore->SetValue(PKEY_Image_Dimensions, pv);
-        }
+            if (SUCCEEDED(hr))
+            {
+                wchar_t szResolution[30];
+                hr = StringCchPrintf(szResolution, ARRAYSIZE(szResolution), L"%i x %i", pImageParser->GetWidth(), pImageParser->GetHeight());
+                pv.vt = VT_LPWSTR;
+                pv.pwszVal = szResolution;
+                hr = _spPropertyStore->SetValue(PKEY_Image_Dimensions, pv);
+            }
 
-        if (SUCCEEDED(hr))
-        {
-            _fPropertiesInitialized = true;
-        }
+            if (SUCCEEDED(hr))
+            {
+                _fPropertiesInitialized = true;
+            }
 
-        delete pImageParser;
-        pImageParser = nullptr;
+            delete pImageParser;
+            pImageParser = nullptr;
 
-        IStream_Reset(_spStream);
+            IStream_Reset(_spStream);
+        }
     }
     return hr;
 }

PIFShellExtensionsTests/PropertyStoreTests.cpp

@@ -41,6 +41,28 @@ namespace PIFShellExtensionsTests
             pPIFShellExt->Release();
         }
 
+        TEST_METHOD(VerifyInvalidFileGetPropertyStore)
+        {
+            CComPtr<IStream> spStream;
+            Assert::IsTrue(SHCreateStreamOnFileEx(SZ_ROOT_TEST_FOLDER L"Invalid\\PGMB\\barbara_zero_width_and_height.pgm", STGM_READ, FILE_ATTRIBUTE_NORMAL, FALSE, NULL, &spStream) == S_OK);
+
+            CPIFShellExt* pPIFShellExt = new CPIFShellExt();
+            Assert::IsNotNull(pPIFShellExt);
+
+            CComPtr<IInitializeWithStream> spInitWithStream;
+            Assert::IsTrue(pPIFShellExt->QueryInterface(IID_PPV_ARGS(&spInitWithStream)) == S_OK);
+
+            Assert::IsTrue(spInitWithStream->Initialize(spStream, STGM_READ) == S_OK);
+
+            CComPtr<IPropertyStore> spPropStore;
+            Assert::IsTrue(pPIFShellExt->QueryInterface(IID_PPV_ARGS(&spPropStore)) == S_OK);
+
+            PROPVARIANT pv = {};
+            Assert::IsFalse(spPropStore->GetValue(PKEY_Image_Dimensions, &pv) == S_OK);
+
+            pPIFShellExt->Release();
+        }
+
 
         // TODO:
         // Verify dimensions are correct