I was writing recently about an MFC bug in CDatabase class in Visual Studio 2012. In the meanwhile I have stumbled upon an even greater bug (check this report on Connect for the details): when the OpenEx function fails (wrong credentials, service not reachable, machine is shot down etc.) it corrupts memory. What happen next is chance. I don’t know when a fix will be available, but until that happens there is a workaround for this. Just like for the previous bug, you have to derive CDatabase and override OpenEx method, copying the original implementation from the MFC sources and adding one more line to null the pointer to already released memory. So, I’ll extend my previous CDatabaseEx class to the following implementation:
class CDatabaseEx : public CDatabase { public: CString GetConnectEx() { CString strConnect = m_strConnect; if (strConnect.GetLength() == 0) { DATA_BLOB connectBlob; if (CryptUnprotectData(&m_blobConnect, NULL, NULL, NULL, NULL, 0, &connectBlob)) { strConnect = (LPTSTR)connectBlob.pbData; LocalFree(connectBlob.pbData); } } return strConnect; } virtual BOOL OpenEx(LPCTSTR lpszConnectString, DWORD dwOptions = 0) { ENSURE_VALID(this); ENSURE_ARG(lpszConnectString == NULL || AfxIsValidString(lpszConnectString)); ENSURE_ARG(!(dwOptions & noOdbcDialog && dwOptions & forceOdbcDialog)); // Exclusive access not supported. ASSERT(!(dwOptions & openExclusive)); m_bUpdatable = !(dwOptions & openReadOnly); TRY { m_strConnect = lpszConnectString; DATA_BLOB connectBlob; connectBlob.pbData = (BYTE *)(LPCTSTR)m_strConnect; connectBlob.cbData = (DWORD)(AtlStrLen(m_strConnect) + 1) * sizeof(TCHAR); if (CryptProtectData(&connectBlob, NULL, NULL, NULL, NULL, 0, &m_blobConnect)) { SecureZeroMemory((BYTE *)(LPCTSTR)m_strConnect, m_strConnect.GetLength() * sizeof(TCHAR)); m_strConnect.Empty(); } // Allocate the HDBC and make connection AllocConnect(dwOptions); if(!Connect(dwOptions)) { m_blobConnect.pbData = NULL; return FALSE; } // Verify support for required functionality and cache info VerifyConnect(); GetConnectInfo(); } CATCH_ALL(e) { // HERE IT IS, the workaround (hopefully temporary) for MFC bug with OpenEx corrupting memory // http://connect.microsoft.com/VisualStudio/feedback/details/760371/localfree-called-twice-in-cdatabase-mfc-11 m_blobConnect.pbData = NULL; Free(); THROW_LAST(); } END_CATCH_ALL return TRUE; } };
You need to modify the code as follows as well at line 47/48:
if(!Connect(dwOptions)) {
m_blobConnect.pbData = NULL;
return FALSE;
}
Otherwise, a failure to connect for reasons other than an exception being caught will also null out the data.
Thanks for pointing that out.
If you call OpenEx() with lpszConnectString pointing to an empty string,
the MFC will open a dialog to let the user choose a new connection.
If that dialog is closed with “Abort” then there will still be a memory
corruption!
I got around this bug with this check:
if( m_strConnect.GetLength() > 0 ) // yet another workaround…
{
DATA_BLOB connectBlob;
connectBlob.pbData = (BYTE *)(LPCTSTR)m_strConnect;
connectBlob.cbData = (DWORD)(AtlStrLen(m_strConnect) + 1) * sizeof(TCHAR);
if (CryptProtectData(&connectBlob, NULL, NULL, NULL, NULL, 0, &m_blobConnect))
{
SecureZeroMemory((BYTE *)(LPCTSTR)m_strConnect, m_strConnect.GetLength() * sizeof(TCHAR));
m_strConnect.Empty();
}
}
regards
Dirk
Unfortunately, if the connection string is not empty but incomplete, the above fix doesn’t work; if the user cancels the source selection dialog double free still occurs. To fix this, we must duplicate the original CDatabase::Connect() function, adding the following fix:
if (m_strConnect.GetLength() == 0)
{
DATA_BLOB connectBlob;
if (CryptUnprotectData(&m_blobConnect, NULL, NULL, NULL, NULL, 0, &connectBlob))
{
m_strConnect = (LPTSTR)connectBlob.pbData;
LocalFree(m_blobConnect.pbData);
m_blobConnect.pbData = NULL; // This is the fix
LocalFree(connectBlob.pbData);
}
}