Bug #91060 Mysql ODBC causes write access violation when using Recordset.Move

• Submitted: 29 May 2018 13:00
• Modified: 13 Sep 2018 18:26
• Reporter: Ciprian Anton
• Status: Closed
• Category: Connector / ODBC
• Severity: S2 (Serious)
• Version: MySQL ODBC 8.0 ANSI Driver 64-bit
• OS: Windows
• CPU Architecture: Any (x64)
• Tags: Write access violation

[29 May 2018 13:00] Ciprian Anton

Description:
When having big VARCHAR columns (e.g. 7000) and using Recordset::Move with a step higher than 7 (20 for e.g.) mysql odbc driver causes access violation. I'm not sure whether the problem comes from mysql odbc or the microsoft code. I confirm that the problem does not reproduce with Microsoft SQLServer2008.

Callstack:
 	myodbc8a.dll!my_wc_mb_latin1(const CHARSET_INFO * cs, unsigned long wc, unsigned char * str, unsigned char * end) Line 324	C++
 	myodbc8a.dll!copy_ansi_result(tagSTMT * stmt, unsigned char * result, __int64 result_bytes, __int64 * avail_bytes, MYSQL_FIELD * field, char * src, unsigned long src_bytes) Line 658	C++
 	myodbc8a.dll!sql_get_data(tagSTMT * stmt, short fCType, unsigned int column_number, void * rgbValue, __int64 cbValueMax, __int64 * pcbValue, char * value, unsigned long length, DESCREC * arrec) Line 531	C++
 	myodbc8a.dll!fill_fetch_buffers(tagSTMT * stmt, char * * values, unsigned int rownum) Line 1840	C++
>	myodbc8a.dll!my_SQLExtendedFetch(void * hstmt, unsigned short fFetchType, __int64 irow, unsigned __int64 * pcrow, unsigned short * rgfRowStatus, bool upd_status) Line 2483	C++
 	myodbc8a.dll!SQLExtendedFetch(void * hstmt, unsigned short fFetchType, __int64 irow, unsigned __int64 * pcrow, unsigned short * rgfRowStatus) Line 2603	C++
 	odbc32.dll!SQLExtendedFetch()	Unknown
 	msdasql.dll!CFetchRowsExtFetch::FetchRows(class CRowset *,struct _GUID const *,__int64,__int64,unsigned __int64,unsigned char *,unsigned __int64 *,unsigned __int64 *,struct tagRowBuff * *)	Unknown
 	msdasql.dll!CImpIRowsetScroll::GetNextRows(unsigned __int64,__int64,__int64,unsigned __int64 *,unsigned __int64 * *)	Unknown
 	msadrh15.dll!00007ffa33d27b82()	Unknown
 	msado15.dll!CRecordset::GetNextRecords(__int64,bool)	Unknown
 	msado15.dll!CRecordset::MoveRelative(__int64)	Unknown
 	msado15.dll!CRecordset::MoveEx(__int64,struct tagVARIANT)	Unknown
 	msado15.dll!CRecordset::Move(long,struct tagVARIANT)	Unknown
 	MyslODBCMoveCrash.exe!ADODB::Recordset15::Move(long NumRecords, const _variant_t & Start) Line 2278	C++
 	MyslODBCMoveCrash.exe!TestRecordSetMove(_com_ptr_t<_com_IIID<ADODB::_Connection,&_GUID_00001550_0000_0010_8000_00aa006d2ea4> > connection, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & tableName) Line 71	C++
 	MyslODBCMoveCrash.exe!main() Line 84	C++
 	MyslODBCMoveCrash.exe!invoke_main() Line 65	C++
 	MyslODBCMoveCrash.exe!__scrt_common_main_seh() Line 253	C++
 	MyslODBCMoveCrash.exe!__scrt_common_main() Line 296	C++
 	MyslODBCMoveCrash.exe!mainCRTStartup() Line 17	C++
 	kernel32.dll!00007ffa44182784()	Unknown
 	ntdll.dll!00007ffa46810d51()	Unknown

I know that I can use MoveNext instead of Move, but doesn't that have a performance penalty?

Also, this seems similar with https://bugs.mysql.com/bug.php?id=68771, but that doesn't seem to be fixed.

How to repeat:
Create a DSN with name: "mysql_odbc_memory_corruption".

Use the following code (I've built it with visual studio 2015) to reproduce:
#import "msado15.dll" rename("EOF", "EndOfFile")

#include <string>
#include <sstream>
#include <iostream>
#include <memory>

using namespace ADODB;
using namespace std;

const char *gConnectionString = "DSN=mysql_odbc_memory_corruption";
const char *gTableName = "large_varchars";
const int gVarCharSize = 7000;
const int gNumRowsToGenerate = 250;

static inline void TESTHR(HRESULT x) { if FAILED(x) _com_issue_error(x); };

static void GenerateVarCharString(char buff[], int buffSize, int row)
{
	int startPos = sprintf_s(buff, buffSize, "%d ", row);	// Add a number for easy tracking
	for (int i = startPos; i < buffSize - 1; i++)
		buff[i] = 'A' + ((i - startPos) % ('Z' - 'A' + 1));
	buff[buffSize - 1] = '\0';
}

static void DropTableIfExist(_ConnectionPtr connection, const std::string &tableName)
{
	connection->Execute(("DROP TABLE IF EXISTS " + tableName).c_str(), nullptr, -1);
}

static void CreateTestTable(_ConnectionPtr connection, const std::string &tableName, int varCharSize)
{
	// Generate sql string
	std::ostringstream sqlStringBuilder;
	sqlStringBuilder << "CREATE TABLE " << tableName << " (";
	sqlStringBuilder << "Col0 VARCHAR(" << varCharSize << ") CHARACTER SET ascii";
	sqlStringBuilder << ")";

	connection->Execute(sqlStringBuilder.str().c_str(), nullptr, -1);
}

static void PopulateTestTable(_ConnectionPtr connection, const std::string &tableName, int numRowsToGenerate, int varCharSize)
{
	_RecordsetPtr recordSet = nullptr;
	TESTHR(recordSet.CreateInstance(__uuidof(Recordset)));

	recordSet->Open(tableName.c_str(), _variant_t((IDispatch *)connection), adOpenKeyset, adLockOptimistic, -1);

	auto valBuff = std::make_unique<char[]>(varCharSize);
	for (int rowIdx = 0; rowIdx < numRowsToGenerate; rowIdx++) {
		GenerateVarCharString(valBuff.get(), varCharSize, rowIdx + 1);
		recordSet->AddNew("Col0", valBuff.get());
	}
}

static void GenerateTestTable(_ConnectionPtr connection, const std::string &tableName)
{
	DropTableIfExist(connection, tableName);
	CreateTestTable(connection, tableName, gVarCharSize);
	PopulateTestTable(connection, tableName, gNumRowsToGenerate, gVarCharSize);
}

static void TestRecordSetMove(_ConnectionPtr connection, const std::string &tableName)
{
	_RecordsetPtr recordSet;
	TESTHR(recordSet.CreateInstance(__uuidof(Recordset)));

	recordSet->Open(("SELECT Col0 FROM " + tableName).c_str(), _variant_t((IDispatch *)connection), adOpenKeyset, adLockOptimistic, -1);
	recordSet->MoveFirst();
	recordSet->Move(20);		// Causes write access violation in mysql odbc driver
}

void main()
{
	CoInitialize(0);

	try {
		_ConnectionPtr connection = nullptr;
		TESTHR(connection.CreateInstance(__uuidof(Connection)));
		connection->Open(gConnectionString, "", "", -1);

		GenerateTestTable(connection, gTableName);
		TestRecordSetMove(connection, gTableName);
	} catch (_com_error &e) {
		cout << "Error:";
		cout << e.ErrorMessage() << std::endl;
		cout << e.Description() << std::endl;
	}

	CoUninitialize();
}

[29 May 2018 13:02] Ciprian Anton

Visual Studio project to reproduce the crash

Attachment: MyslODBCMoveCrash.zip (application/x-zip-compressed, text), 12.42 KiB.

[30 May 2018 6:18] Chiranjeevi Battula

Hello Ciprian Anton,

Thank you for the bug report and testcase.
Verified this behavior on Visual Studio 2017 (C#.Net) and Connector / ODBC 8.0.11.

Thanks,
Chiranjeevi.

[30 May 2018 6:23] Chiranjeevi Battula

myodbc8a.dll!my_wc_mb_latin1(const CHARSET_INFO * cs, unsigned long wc, unsigned char * str, unsigned char * end) Line 324	C++
 	myodbc8a.dll!copy_ansi_result(tagSTMT * stmt, unsigned char * result, __int64 result_bytes, __int64 * avail_bytes, MYSQL_FIELD * field, char * src, unsigned long src_bytes) Line 658	C++
 	myodbc8a.dll!sql_get_data(tagSTMT * stmt, short fCType, unsigned int column_number, void * rgbValue, __int64 cbValueMax, __int64 * pcbValue, char * value, unsigned long length, DESCREC * arrec) Line 531	C++
 	myodbc8a.dll!fill_fetch_buffers(tagSTMT * stmt, char * * values, unsigned int rownum) Line 1840	C++
 	myodbc8a.dll!my_SQLExtendedFetch(void * hstmt, unsigned short fFetchType, __int64 irow, unsigned __int64 * pcrow, unsigned short * rgfRowStatus, bool upd_status) Line 2483	C++
 	myodbc8a.dll!SQLExtendedFetch(void * hstmt, unsigned short fFetchType, __int64 irow, unsigned __int64 * pcrow, unsigned short * rgfRowStatus) Line 2603	C++
 	[External Code]	
 	msdasql.dll!00007ffc9751c784()	Unknown
 	msdasql.dll!00007ffc974f72d4()	Unknown
 	msadrh15.dll!00007ffc942c2d09()	Unknown
 	msado15.dll!00007ffc97851638()	Unknown
 	msado15.dll!00007ffc978573bd()	Unknown
 	msado15.dll!00007ffc97927c85()	Unknown
 	msado15.dll!00007ffc979270fa()	Unknown
>	MyslODBCMoveCrash.exe!ADODB::Recordset15::Move(long NumRecords, const _variant_t & Start) Line 2278	C++
 	MyslODBCMoveCrash.exe!TestRecordSetMove(_com_ptr_t<_com_IIID<ADODB::_Connection,&_GUID_00001550_0000_0010_8000_00aa006d2ea4> > connection, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & tableName) Line 71	C++
 	MyslODBCMoveCrash.exe!main() Line 84	C++
 	[External Code]

[30 May 2018 6:23] Chiranjeevi Battula

Screenshot

Attachment: Bug_91060.JPG (image/jpeg, text), 181.88 KiB.

[7 Jun 2018 6:28] Bogdan Degtyariov

Posted by developer:
 
As testing has revealed the problem occurs because of the following:

ADODB binds results buffers and reads some data.
Then it sets SQL_ATTR_RETRIEVE_DATA statement attribute to SQL_RD_OFF, which should turn off reading data into the buffers.
After this is done the result buffers are reduced or reallocated.
Then ADODB requests reading more rows from the table.
MySQL ODBC driver ignores SQL_RD_OFF value for SQL_ATTR_RETRIEVE_DATA attribute and keeps writing into the data buffers.
This causes write access violation errors.

[13 Sep 2018 18:26] Philip Olson

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

Because the MySQL ODBC driver ignored the SQL_RD_OFF value for the
SQL_ATTR_RETRIEVE_DATA attribute, it incorrectly kept writing into the
data buffers. This led to write access violation errors when data was
written into the buffer when the user application explicitly requested not
to write there.

Thank you for the bug report.

Also thanks to Bogdan for clarifying the fix; oh, and also for fixing :)