[PATCH 0/1] MR8298: ole32: Don't get metafile extent if there is no metafile.

[Yuxuan Shui (＠yshui)]

If entry->stgmedium.hEnhMetaFile is NULL, the data_size we get will be 0, we
then allocate 0 bytes for data, but expect it to contain at least a
METAFILEPICT, resulting in reading out-of-bound.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8298

[PATCH 1/1] ole32: Don't get metafile extent if there is no metafile.

[Yuxuan Shui]

From: Yuxuan Shui <yshui(a)codeweavers.com>

If entry->stgmedium.hEnhMetaFile is NULL, the data_size we get will be 0, we
then allocate 0 bytes for data, but expect it to contain at least a
METAFILEPICT, resulting in reading out-of-bound.
---
 dlls/ole32/datacache.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/dlls/ole32/datacache.c b/dlls/ole32/datacache.c
index 97b4c9cffca..0a7aaf1a7cf 100644
--- a/dlls/ole32/datacache.c
+++ b/dlls/ole32/datacache.c
@@ -1030,21 +1030,28 @@ static HRESULT save_emf(DataCacheEntry *entry, BOOL contents, IStream *stream)
         }
         data_size = GetWinMetaFileBits(entry->stgmedium.hEnhMetaFile, 0, NULL, MM_ANISOTROPIC, hdc);
         header.dwSize = data_size;
-        data = HeapAlloc(GetProcessHeap(), 0, header.dwSize);
-        if (!data)
+        if (data_size)
         {
-            ReleaseDC(0, hdc);
-            return E_OUTOFMEMORY;
+            data = HeapAlloc(GetProcessHeap(), 0, header.dwSize);
+            if (!data)
+            {
+                ReleaseDC(0, hdc);
+                return E_OUTOFMEMORY;
+            }
+            GetWinMetaFileBits(entry->stgmedium.hEnhMetaFile, header.dwSize, data, MM_ANISOTROPIC, hdc);
+            mfpict = (METAFILEPICT *)data;
+            header.dwObjectExtentX = mfpict->xExt;
+            header.dwObjectExtentY = mfpict->yExt;
         }
-        GetWinMetaFileBits(entry->stgmedium.hEnhMetaFile, header.dwSize, data, MM_ANISOTROPIC, hdc);
+        else
+            header.dwObjectExtentX = header.dwObjectExtentY = 0;
         ReleaseDC(0, hdc);
-        mfpict = (METAFILEPICT *)data;
-        header.dwObjectExtentX = mfpict->xExt;
-        header.dwObjectExtentY = mfpict->yExt;
         hr = IStream_Write(stream, &header, sizeof(PresentationDataHeader), NULL);
-        if (hr == S_OK && data_size)
-            hr = IStream_Write(stream, data, data_size, NULL);
-        HeapFree(GetProcessHeap(), 0, data);
+        if (data_size)
+        {
+            if (hr == S_OK) hr = IStream_Write(stream, data, data_size, NULL);
+            HeapFree(GetProcessHeap(), 0, data);
+        }
     }
     else if (entry->stgmedium.tymed != TYMED_NULL)
     {
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/8298

[Nikolay Sivov (＠nsivov)]

Could we instead check for entry->stgmedium.hEnhMetaFile being null? And if it's null the next question is if we need to write anything at all to begin with.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8298#note_106393

[Yuxuan Shui (＠yshui)]

On Wed Aug 27 14:01:55 2025 +0000, Nikolay Sivov wrote:
> Could we instead check for entry->stgmedium.hEnhMetaFile being null? And
> if it's null the next question is if we need to write anything at all to
> begin with.
> we have [tests](https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/ole32/tests/ole2.c?re...) that calls `IPersistStorage_Save` on data cache without calling `IOleCache2_SetData`. and those tests do check the data written IIUC. so the current behavior is correct besides the oob

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8298#note_113982