Skip to content

Commit

Permalink
Fix GH-17658: COMPersistHelper::LoadFromStream() can segfault
Browse files Browse the repository at this point in the history
The actual fix is trivial, but to be able to test the behavior we have
to introduce an own COM object, since existing persistable objects
likely implement `IPersistInit`, not only `IPersist`.  We also want to
avoid further test dependencies on possibly unavailable objects, such
as `Word.Application`.

To this purposes, we add a small COM in-process server, which may be
extended for other testing purposes.  We keep it simple by implementing
it in C++, but without using any more sophisticated frameworks like ATL.

This component needs to be built explicitly (`nmake comtest.dll`), and
also needs to be explicitly registered (`nmake register_comtest`).
When no longer needed, it is possible to unregister the component
(`nmake unregister_comtest`).
  • Loading branch information
cmb69 committed Jan 31, 2025
1 parent 16c9652 commit 7a78492
Show file tree
Hide file tree
Showing 9 changed files with 409 additions and 1 deletion.
1 change: 1 addition & 0 deletions .github/scripts/windows/build_task.bat
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ cmd /c configure.bat ^
if %errorlevel% neq 0 exit /b 3

nmake /NOLOGO
nmake /NOLOGO comtest.dll
if %errorlevel% neq 0 exit /b 3

exit /b 0
4 changes: 4 additions & 0 deletions .github/scripts/windows/test_task.bat
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ hMailServer.exe /verysilent
cd %APPVEYOR_BUILD_FOLDER%
%PHP_BUILD_DIR%\php.exe -dextension_dir=%PHP_BUILD_DIR% -dextension=com_dotnet .github\setup_hmailserver.php

rem prepare for com_dotnet
nmake register_comtest

mkdir %PHP_BUILD_DIR%\test_file_cache
rem generate php.ini
echo extension_dir=%PHP_BUILD_DIR% > %PHP_BUILD_DIR%\php.ini
Expand Down Expand Up @@ -146,6 +149,7 @@ nmake test TESTS="%OPCACHE_OPTS% -g FAIL,BORK,LEAK,XLEAK %ASAN_OPTS% --no-progre

set EXIT_CODE=%errorlevel%

nmake unregister_comtest
taskkill /f /im snmpd.exe

exit /b %EXIT_CODE%
30 changes: 30 additions & 0 deletions ext/com_dotnet/Makefile.frag.w32
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
$(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest_i.c: ext\com_dotnet\tests\comtest\comtest.idl
md $(BUILD_DIR)\ext\com_dotnet\tests\comtest
midl /nologo /h $(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest.h /iid $(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest_i.c /tlb $(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest.tlb ext\com_dotnet\tests\comtest\comtest.idl

$(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest.obj $(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest_i.obj: ext\com_dotnet\tests\comtest\comtest.cpp $(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest_i.c
$(PHP_CL) /nologo /c /Fo:$(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest.obj /I $(BUILD_DIR)\ext\com_dotnet\tests\comtest ext\com_dotnet\tests\comtest\comtest.cpp
$(PHP_CL) /nologo /c /Fo:$(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest_i.obj $(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest_i.c

$(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest.dll: $(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest.obj $(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest_i.obj ext\com_dotnet\tests\comtest\comtest.def
$(LINK) /nologo /dll /out:$(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest.dll $(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest.obj $(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest_i.obj /def:ext\com_dotnet\tests\comtest\comtest.def OleAut32.lib

comtest.dll: $(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest.dll

register_comtest:
@reg add HKCU\SOFTWARE\Classes\TypeLib\{AE8685BE-3758-4BDA-91DB-1459EBA24747} /f > NUL
@reg add HKCU\SOFTWARE\Classes\TypeLib\{AE8685BE-3758-4BDA-91DB-1459EBA24747}\1.0 /d "PHP COM Test Library" /f > NUL
@reg add HKCU\SOFTWARE\Classes\TypeLib\{AE8685BE-3758-4BDA-91DB-1459EBA24747}\1.0\0 /f > NUL
@reg add HKCU\SOFTWARE\Classes\TypeLib\{AE8685BE-3758-4BDA-91DB-1459EBA24747}\1.0\0\win32 /d "$(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest.tlb" /f > NUL
@reg add HKCU\SOFTWARE\Classes\TypeLib\{AE8685BE-3758-4BDA-91DB-1459EBA24747}\1.0\0\win64 /d "$(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest.tlb" /f > NUL
@reg add HKCU\SOFTWARE\Classes\TypeLib\{AE8685BE-3758-4BDA-91DB-1459EBA24747}\1.0\FLAGS /d 0 /f > NUL
@reg add HKCU\SOFTWARE\Classes\TypeLib\{AE8685BE-3758-4BDA-91DB-1459EBA24747}\1.0\HELPDIR /d "$(BUILD_DIR)\ext\com_dotnet\tests\comtest" /f > NUL
@reg add HKCU\SOFTWARE\Classes\CLSID\{B13FE324-D595-44C7-97D7-82CE20EDF878} /d "PHP COM Test Document" /f > NUL
@reg add HKCU\SOFTWARE\Classes\CLSID\{B13FE324-D595-44C7-97D7-82CE20EDF878}\InprocServer32 /d "$(BUILD_DIR)\ext\com_dotnet\tests\comtest\comtest.dll" /f > NUL
@reg add HKCU\SOFTWARE\Classes\PHP.Test.Document /d "PHP COM Test Document" /f > NUL
@reg add HKCU\SOFTWARE\Classes\PHP.Test.Document\CLSID /d "{B13FE324-D595-44C7-97D7-82CE20EDF878}" /f > NUL

unregister_comtest:
-@reg delete HKCU\SOFTWARE\Classes\PHP.Test.Document /f > NUL
-@reg delete HKCU\SOFTWARE\Classes\CLSID\{B13FE324-D595-44C7-97D7-82CE20EDF878} /f > NUL
-@reg delete HKCU\SOFTWARE\Classes\TypeLib\{AE8685BE-3758-4BDA-91DB-1459EBA24747} /f > NUL
2 changes: 1 addition & 1 deletion ext/com_dotnet/com_persist.c
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ CPH_METHOD(LoadFromStream)
} else {
res = get_persist_stream(helper);
if (helper->ips) {
res = IPersistStreamInit_Load(helper->ipsi, stm);
res = IPersistStream_Load(helper->ips, stm);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions ext/com_dotnet/config.w32
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ if (PHP_COM_DOTNET == "yes") {
null, "/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1");
AC_DEFINE('HAVE_COM_DOTNET', 1, "Define to 1 if the PHP extension 'com_dotnet' is available.");
CHECK_HEADER_ADD_INCLUDE('mscoree.h', 'CFLAGS_COM_DOTNET');
ADD_MAKEFILE_FRAGMENT();
}
318 changes: 318 additions & 0 deletions ext/com_dotnet/tests/comtest/comtest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,318 @@
/*
+----------------------------------------------------------------------+
| Copyright (c) The PHP Group |
+----------------------------------------------------------------------+
| This source file is subject to version 3.01 of the PHP license, |
| that is bundled with this package in the file LICENSE, and is |
| available through the world-wide-web at the following url: |
| https://www.php.net/license/3_01.txt |
| If you did not receive a copy of the PHP license and are unable to |
| obtain it through the world-wide-web, please send a note to |
| license@php.net so we can mail you a copy immediately. |
+----------------------------------------------------------------------+
| Author: Christoph M. Becker <cmb@php.net> |
| Based on: <https://thrysoee.dk/InsideCOM+/> |
+----------------------------------------------------------------------+
*/

#include "comtest.h" // Generated by MIDL

long g_cComponents = 0;
long g_cLocks = 0;

class CDocument : public IDocument, IPersistStream
{
public:
// IUnknown
STDMETHODIMP_(ULONG) AddRef();
STDMETHODIMP_(ULONG) Release();
STDMETHODIMP QueryInterface(REFIID riid, void** ppv);

// IDispatch
STDMETHODIMP GetTypeInfoCount(UINT* pCountTypeInfo);
STDMETHODIMP GetTypeInfo(UINT iTypeInfo, LCID lcid, ITypeInfo** ppITypeInfo);
STDMETHODIMP GetIDsOfNames(REFIID riid, LPOLESTR* rgszNames, UINT cNames, LCID lcid, DISPID* rgDispId);
STDMETHODIMP Invoke(DISPID dispIdMember, REFIID riid, LCID lcid, WORD wFlags, DISPPARAMS* pDispParams,
VARIANT* pVarResult, EXCEPINFO* pExcepInfo, UINT* puArgErr);

// IDocument
STDMETHODIMP get_Content(BSTR* retvalue);
STDMETHODIMP put_Content(BSTR newvalue);

// IPersist
STDMETHODIMP GetClassID(CLSID *pClassID);

// IPersistStream
STDMETHODIMP IsDirty( void);
STDMETHODIMP Load(IStream *pStm);
STDMETHODIMP Save(IStream *pStm, BOOL fClearDirty);
STDMETHODIMP GetSizeMax(ULARGE_INTEGER *pcbSize);

CDocument() : m_content(SysAllocString(L"")), m_cRef(1) { g_cComponents++; }
~CDocument() { SysFreeString(m_content); g_cComponents--; }
HRESULT Init(void);

private:
BSTR m_content;
ULONG m_cRef;
ITypeInfo* m_pTypeInfo;
BOOL m_dirty;
};

ULONG CDocument::AddRef()
{
return ++m_cRef;
}

ULONG CDocument::Release()
{
if (--m_cRef != 0) {
return m_cRef;
}
m_pTypeInfo->Release();
delete this;
return 0;
}

HRESULT CDocument::QueryInterface(REFIID riid, void** ppv)
{
if (riid == IID_IUnknown) {
*ppv = (IDocument*) this;
} else if (riid == IID_IDocument) {
*ppv = (IDocument *) this;
} else if (riid == IID_IDispatch) {
*ppv = (IDispatch*) this;
} else if (riid == IID_IPersistStream) {
*ppv = (IPersistStream *) this;
} else {
*ppv = NULL;
return E_NOINTERFACE;
}
AddRef();
return S_OK;
}

HRESULT CDocument::GetTypeInfoCount(UINT* pCountTypeInfo)
{
*pCountTypeInfo = 1;
return S_OK;
}

HRESULT CDocument::GetTypeInfo(UINT iTypeInfo, LCID lcid, ITypeInfo** ppITypeInfo)
{
*ppITypeInfo = NULL;
if (iTypeInfo != 0) {
return DISP_E_BADINDEX;
}
m_pTypeInfo->AddRef();
*ppITypeInfo = m_pTypeInfo;
return S_OK;
}

HRESULT CDocument::GetIDsOfNames(REFIID riid, LPOLESTR* rgszNames, UINT cNames, LCID lcid, DISPID* rgDispId)
{
if (riid != IID_NULL) {
return DISP_E_UNKNOWNINTERFACE;
}
return DispGetIDsOfNames(m_pTypeInfo, rgszNames, cNames, rgDispId);
}

HRESULT CDocument::Invoke(DISPID dispIdMember, REFIID riid, LCID lcid, WORD wFlags, DISPPARAMS* pDispParams,
VARIANT* pVarResult, EXCEPINFO* pExcepInfo, UINT* puArgErr)
{
if (riid != IID_NULL) {
return DISP_E_UNKNOWNINTERFACE;
}
return DispInvoke(this, m_pTypeInfo, dispIdMember, wFlags, pDispParams, pVarResult, pExcepInfo, puArgErr);
}

HRESULT CDocument::get_Content(BSTR* retvalue)
{
*retvalue = SysAllocString(m_content);
return S_OK;
}

HRESULT CDocument::put_Content(BSTR newvalue)
{
SysFreeString(m_content);
m_content = SysAllocString(newvalue);
return S_OK;
}

HRESULT CDocument::Init(void)
{
ITypeLib* pTypeLib;
if (FAILED(LoadRegTypeLib(LIBID_ComTest, 1, 0, LANG_NEUTRAL, &pTypeLib))) {
return E_FAIL;
}
HRESULT hr = pTypeLib->GetTypeInfoOfGuid(IID_IDocument, &m_pTypeInfo);
if (FAILED(hr)) {
pTypeLib->Release();
return hr;
}

pTypeLib->Release();
return S_OK;
}

HRESULT CDocument::GetClassID(CLSID *pClassID)
{
*pClassID = CLSID_Document;
return S_OK;
}

HRESULT CDocument::IsDirty(void)
{
return m_dirty ? S_OK : S_FALSE;
}

HRESULT CDocument::Load(IStream *pStm)
{
ULONG read = 0;
ULONG cbSize;
char *string;
if (FAILED(pStm->Read(&cbSize, sizeof cbSize, &read))) {
return S_FALSE;
}
string = (char *) malloc(cbSize);
if (string == NULL) {
return S_FALSE;
}
if (FAILED(pStm->Read(string, cbSize, &read))) {
return S_FALSE;
}
SysFreeString(m_content);
m_content = SysAllocStringByteLen(string, cbSize);

m_dirty = FALSE;

return S_OK;
}

HRESULT CDocument::Save(IStream *pStm, BOOL fClearDirty)
{
ULONG written = 0;
ULONG cbSize = SysStringByteLen(m_content);
HRESULT hr;
if (FAILED(hr = pStm->Write(&cbSize, sizeof cbSize, &written))) {
return S_FALSE;
}
if (FAILED(hr = pStm->Write(m_content, cbSize, &written))) {
return S_FALSE;
}

if (fClearDirty) {
m_dirty = FALSE;
}

return S_OK;
}

HRESULT CDocument::GetSizeMax(ULARGE_INTEGER *pcbSize)
{
(*pcbSize).QuadPart = sizeof(ULONG) + SysStringByteLen(m_content);
return S_OK;
}

class CDocumentFactory : public IClassFactory
{
public:
// IUnknown
STDMETHODIMP_(ULONG) AddRef();
STDMETHODIMP_(ULONG) Release();
STDMETHODIMP QueryInterface(REFIID riid, void** ppv);

// IClassFactory
STDMETHODIMP CreateInstance(IUnknown* pUnknownOuter, REFIID riid, void** ppv);
STDMETHODIMP LockServer(BOOL bLock);

CDocumentFactory() : m_cRef(1) { g_cLocks++; }
~CDocumentFactory() { g_cLocks--; }

private:
ULONG m_cRef;
};

ULONG CDocumentFactory::AddRef()
{
return ++m_cRef;
}

ULONG CDocumentFactory::Release()
{
if (--m_cRef != 0) {
return m_cRef;
}
delete this;
return 0;
}

HRESULT CDocumentFactory::QueryInterface(REFIID riid, void** ppv)
{
if (riid == IID_IUnknown) {
*ppv = (IUnknown*)this;
} else if (riid == IID_IClassFactory) {
*ppv = (IClassFactory*)this;
} else {
*ppv = NULL;
return E_NOINTERFACE;
}
AddRef();
return S_OK;
}

HRESULT CDocumentFactory::CreateInstance(IUnknown *pUnknownOuter, REFIID riid, void** ppv)
{
if (pUnknownOuter != NULL) {
return CLASS_E_NOAGGREGATION;
}

CDocument *pDocument = new CDocument;
if (pDocument == NULL) {
return E_OUTOFMEMORY;
}

HRESULT hr;
if (FAILED(hr = pDocument->Init())) {
return hr;
}
hr = pDocument->QueryInterface(riid, ppv);
pDocument->Release();
return hr;
}

HRESULT CDocumentFactory::LockServer(BOOL bLock)
{
if (bLock) {
g_cLocks++;
} else {
g_cLocks --;
}
return S_OK;
}

HRESULT __stdcall DllCanUnloadNow()
{
if (g_cLocks == 0) {
return S_OK;
} else {
return S_FALSE;
}
}

HRESULT __stdcall DllGetClassObject(REFCLSID clsid, REFIID riid, void** ppv)
{
if (clsid != CLSID_Document) {
return CLASS_E_CLASSNOTAVAILABLE;
}

CDocumentFactory* pFactory = new CDocumentFactory;
if (pFactory == NULL) {
return E_OUTOFMEMORY;
}

// riid is probably IID_IClassFactory.
HRESULT hr = pFactory->QueryInterface(riid, ppv);
pFactory->Release();
return hr;
}
4 changes: 4 additions & 0 deletions ext/com_dotnet/tests/comtest/comtest.def
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
LIBRARY comtest.dll
EXPORTS
DllGetClassObject PRIVATE
DllCanUnloadNow PRIVATE
Loading

0 comments on commit 7a78492

Please sign in to comment.