Bug #88371 Calling MySQLDrivercConnect with a NULL pcbConnStrOut causes a crash

• Submitted: 6 Nov 2017 6:20
• Modified: 21 Dec 2017 19:32
• Reporter: Marcin Zawiejski
• Status: Closed
• Category: Connector / ODBC
• Severity: S3 (Non-critical)
• Version: 5.3.9
• OS: Windows
• CPU Architecture: Any

[6 Nov 2017 6:20] Marcin Zawiejski

Description:
MySQLDriverConnect (driver/connect.c) writes to *pcbConnStrOut while the pcbConnStrOut pointer is not checked for a NULL value. This causes a crash because of an access violation if the pointer is NULL.

The relevant code is:

    if (szConnStrOut)
    {
      *pcbConnStrOut= (SQLSMALLINT)myodbc_min(cbConnStrOutMax, *pcbConnStrOut);
      memcpy(szConnStrOut, prompt_outstr, (size_t)*pcbConnStrOut*sizeof(SQLWCHAR));
      /* term needed if possibly truncated */
      szConnStrOut[*pcbConnStrOut - 1] = 0;
    }

How to repeat:
Call MySQLDriverConnect function with pcbConnStrOut argument set to NULL. This is also triggered indirectly by a Microsoft OLE DB Provider for ODBC Drivers that sets the value to NULL when calling IDBInitialize::Initialize.

Suggested fix:
Check for a NULL pcbConnStrOut before writing. There are a few other instances of "if (pcbConnStrOut)" in the method so I believe it is expected that the pointer can be NULL.

[7 Nov 2017 11:38] Marcin Zawiejski

Here's a program that reproduces this:
1. Compile and run
2. Select a preconfigured Machine Data Source for "MySQL ODBC 5.3 Unicode Driver"
3. Fill in details in "MySQL Connector/ODBC Data Source Configuration" and press "OK" -> this causes the access violation

-- code:

#include <atlbase.h>
#include <msdasc.h>

HRESULT Test()
{
	HRESULT hr{};

	CComPtr<IDataInitialize> DataInitialize;
	CComPtr<IDBInitialize> DBInitialize;
	CComPtr<IDBProperties> DBProperties;

	hr = CoCreateInstance(CLSID_MSDAINITIALIZE, NULL, CLSCTX_INPROC_SERVER, IID_IDataInitialize, (void**)&DataInitialize);
	if (FAILED(hr))
		goto Quit;

	CLSID clsidSource;

	hr = CLSIDFromString(L"{c8b522cb-5cf3-11ce-ade5-00aa0044773d}", &clsidSource);
	if (FAILED(hr))
		goto Quit;

	hr = DataInitialize->CreateDBInstance(clsidSource, NULL, CLSCTX_INPROC_SERVER, NULL, IID_IDBInitialize, (IUnknown**)&DBInitialize);
	if (FAILED(hr))
		goto Quit;

	DBPROP rgProperties[2];

	rgProperties[0].colid = DB_NULLID;
	rgProperties[0].dwOptions = DBPROPOPTIONS_REQUIRED;
	rgProperties[0].dwPropertyID = DBPROP_INIT_PROMPT;
	rgProperties[0].dwStatus = DBPROPSTATUS_OK;
	V_VT(&rgProperties[0].vValue) = VT_I2;
	V_I2(&rgProperties[0].vValue) = 1;

	rgProperties[1].colid = DB_NULLID;
	rgProperties[1].dwOptions = DBPROPOPTIONS_REQUIRED;
	rgProperties[1].dwPropertyID = DBPROP_INIT_HWND;
	rgProperties[1].dwStatus = DBPROPSTATUS_OK;
	V_VT(&rgProperties[1].vValue) = VT_I4;
	V_I4(&rgProperties[1].vValue) = (LONG)GetDesktopWindow();

	DBPROPSET rgPropSet[1];
	rgPropSet[0].cProperties = 2;
	rgPropSet[0].guidPropertySet = DBPROPSET_DBINIT;
	rgPropSet[0].rgProperties = rgProperties;

	hr = DBInitialize.QueryInterface(&DBProperties);
	if (FAILED(hr))
		goto Quit;

	hr = DBProperties->SetProperties(1, rgPropSet);
	if (FAILED(hr))
		goto Quit;

	hr = DBInitialize->Initialize();
	if (FAILED(hr))
		goto Quit;

Quit:

	return hr;
}

int main()
{
	CoInitialize(0);
	Test();
	CoUninitialize();

    return 0;
}

-- stack trace:

>	myodbc5w.dll!MySQLDriverConnect(void * hdbc=0x01188628, HWND__ * hwnd=0x00010010, unsigned short * szConnStrIn=0x040095a8, short cbConnStrIn=-3, unsigned short * szConnStrOut=0x011740d8, short cbConnStrOutMax=1024, short * pcbConnStrOut=0x00000000, unsigned short fDriverCompletion=2) Line 857	C	Symbols loaded.
 	odbc32.dll!_SQLInternalDriverConnectW@40()	Unknown	Symbols loaded.
 	odbc32.dll!_SQLDriverConnectW@32()	Unknown	Symbols loaded.
 	msdasql.dll!CODBCHandle::OHDriverConnect(class CHdbcNode *,void *,unsigned short const *,short,unsigned short,class CImpISQLRequestDiagFields *)	Unknown	Symbols loaded.
 	msdasql.dll!CImpIDBInitialize::Initialize(void)	Unknown	Symbols loaded.
 	oledb32.dll!CDBInitialize::DoInitialize(struct IDBInitialize *)	Unknown	Symbols loaded.
 	oledb32.dll!CDBInitialize::Initialize(void)	Unknown	Symbols loaded.
 	oledb32.dll![thunk]:ATL::CComObject<class CSCDispMan>::Release`adjustor{4}' (void)	Unknown	Symbols loaded.
 	ConsoleApplication6.exe!Test() Line 59	C++	Symbols loaded.
 	ConsoleApplication6.exe!main() Line 72	C++	Symbols loaded.
 	ConsoleApplication6.exe!invoke_main() Line 78	C++	Symbols loaded.
 	ConsoleApplication6.exe!__scrt_common_main_seh() Line 283	C++	Symbols loaded.
 	ConsoleApplication6.exe!__scrt_common_main() Line 326	C++	Symbols loaded.
 	ConsoleApplication6.exe!mainCRTStartup() Line 17	C++	Symbols loaded.
 	kernel32.dll!@BaseThreadInitThunk@12()	Unknown	Symbols loaded.
 	ntdll.dll!__RtlUserThreadStart()	Unknown	Symbols loaded.
 	ntdll.dll!__RtlUserThreadStart@8()	Unknown	Symbols loaded.

[10 Nov 2017 6:17] Bogdan Degtyariov

Hi Marcin,

Thank you for your report.
The issue is verified.

[21 Dec 2017 19:32] Philip Olson

Posted by developer:
 
Fixed as of the upcoming MySQL Connector/ODBC 5.3.10 release, and here's the changelog entry:

Calling MySQLDriverConnect with the pcbConnStrOut argument set to NULL
caused an unexpected failure.

Thank you for the bug report.