Memory corruption and leaks in AtlIPersistPropertyBag_Load
The VS2005 atlcom.h attempts to fix some of problems in AtlIPersistPropertyBag_Load that were present in VS2003, but only makes it fail in a different way.
The first problem occurs when BSTR properties are being enumerated - the CComVariant used in the loop isn't being cleared before being overwritten with the next property - fine for UI4's but not for BSTR's.
The new version calls var.Clear() to deal with this, however does not take account of the fact that (after releasing the data) VariantClear only resets the type - the value field is still set to the old released value:
var.Clear();
.vt;void* pData = (void*) (pMap
.dwOffsetData + (DWORD_PTR)pThis);HRESULT hr = pPropBag->Read(pMap
.szDesc, &var, pErrorLog);and
var.Clear();
var.vt = vt;
HRESULT hr = pPropBag->Read(pMap.szDesc, &var, pErrorLog);
In the case of a BSTR, for example, the in/out var parameter for Read() gets marshalled as though it is a valid BSTR, when the bstrVal is in fact an old released pointer.
The result is unpredictable, but in my module it causes stack corruption and general strangeness with BSTRs in the process.
Since the variant type could be anything, the solution seems to be to zero the variant struct before setting the new vt:
var.Clear();
ZeroMemory((VARIANT *)&var,
The second problem involves overwriting of raw BSTR properties. The old value in the property map is not being released, causing a memory leak:
case VT_BSTR:*((BSTR*)pData) = ::SysAllocString(var.bstrVal);
Should be:
case VT_BSTR:
if(*((BSTR*)pData)) ::SysFreeString(*((BSTR*)pData));
*((BSTR*)pData) = ::SysAllocString(var.bstrVal);
