Bug #87248 MyODBC Connector on Macintosh Crashes with Address Sanitizer
Submitted: 29 Jul 2017 22:38 Modified: 24 Oct 2017 5:01
Reporter: Howard Rodstein Email Updates:
Status: Analyzing Impact on me:
None 
Category:Connector / ODBC Severity:S2 (Serious)
Version:5.3.9 OS:Mac OS X (10.11.6)
Assigned to: Assigned Account CPU Architecture:Any

[29 Jul 2017 22:38] Howard Rodstein
Description:
Executing this query while running under Address Sanitizer causes Address Sanitizer to flag a buffer read violation (reading unallocated memory) in the strlen function:

SELECT orderID,orderDate,customerRefNum,orderStatus from orderTable WHERE customerID=0 AND (orderType='Standard' OR orderType IS NULL) Order by orderID

Removing " OR orderType IS NULL" eliminates the crash.

The stack crawl at the time of the crash is:
<client code>		// In client program
SQLFetch		// libiodbc.2.dylib
SQLFetch_Internal	// libiodbc.2.dylib
SQLExtendedFetch	// libmyodbc5a.so
fill_fetch_buffers	// libmyodbc5a.so
wrap_strlen		// libclang_rt.asan_osx_dynamic.dylib

The buffer where the violation is occurring is not memory that the client allocated.

This kind of Address Sanitizer violation in strlen usually indicates accessing unallocated or uninitialized memory or on text that is not null-terminated.

If Address Sanitizer is turned off, there is no crash.

The client is our program, Igor Pro. It uses ODBC. We are accessing our MySQL server via MyODBC on Macintosh, 64-bit.
 

How to repeat:
You would have to create a MySQL database on which you can do a query like the one above. You would have to access the database via MyODBC with Address Sanitizer turned on. This means your client would have to be running under Xcode.

I suspect that this would also crash on Unix or Windows if running Address Sanitizer.

The database that we are accessing contains customer information that we have to keep private so I can't give you access to it.

Suggested fix:
If you look at the code for fill_fetch_buffers and consider how it is affected by the presence of "OR orderType IS NULL", you may find that you are calling strlen on unallocated or uninitialized memory or on text that is not null-terminated.
[10 Aug 2017 12:13] Chiranjeevi Battula
Hello  Howard Rodstein,

Thank you for the bug report.
Per the MySQL Support Lifecycle policy regarding ending support for 3.5x versions that have reached end of life, we plan to discontinue building 3.5x binaries for the connector ODBC.

Thanks,
Chiranjeevi.
[10 Aug 2017 22:57] Howard Rodstein
I'm not sure where the 3.52.12 version number came from which was in my original post.

I actually see the problem in Connector/ODBC 5.3.9 which is the currently available version from https://dev.mysql.com/downloads/connector/odbc/. It is installed on my Macintosh at "/usr/local/mysql-connector-odbc-5.3.9-macos10.12-x86-64bit".

I am using the ANSI version identified in iODBC Data Source Administrator as "MySQL ODBC 5.3 ANSI Driver, version 05.03.0009".
[25 Sep 2017 7:28] Bogdan Degtyariov
Hi Howard,

Thank you for your bug report.
First of all, we need to adjust the severity of this bug.
It is not S1 (Represents a complete loss of service, a significant functionality is missing, a system that hangs indefinitely; and there is no available workaround.) because it works without the sanitizer. Therefore, setting S2.

You are right, strlen() function can be dangerous if used on strings not terminated by 0x00 byte. However, in that particular case the logic of ODBC is this:

 1. Client application can supply the length of the string.
    In this case strlen() is not used.

 2. Client application can give SQL_NTS instead of the string length
    and in this way indicate that the string is null-terminated.
    In this case strlen() is used. There is no way of checking the buffer
    boundaries if client specified it wrong.

However, in this situation I believe it could be different since it happens at the stage of reading the results.

Unfortunately, it is hard to come to any conclusions using just the query you specified. Would it be possible for you to make a short independent test case?

If not, perhaps a simplified data and SQL query where we could try it on our side.
Thanks.
[24 Oct 2017 0:20] Howard Rodstein
We have come up with a simplified table. We can reproduce the crash using the query shown above with this table. I will attach the table.

Here are further details.

As a reminder, we see this crash on Macintosh only when our executable is compiled with Address Sanitizer.

We are binding one parameter using SQLBindParameter to the customerid field. This binding seems to be required to induce the crash. The customerid field is defined in the database as follows:
{{{
customerid	int(11)	NO
}}}
We bind this to a C variable of type double.

We are fetching into four columns defined in the database as follows:
{{{
Column 0:	orderid		int(11)		bound to SQL_C_SLONG (SQLINTEGER - 32-bit signed integer)	// 4 byte buffer
Column 1:	orderDate	date		bound to SQL_TYPE_DATE (DATE_STRUCT - 6 bytes)         // 6 byte buffer
Column 2:	customerRefNum	varchar(45)	bound to SQL_CHAR					// 136 byte buffer
Column 3:	orderStatus	varchar(45)	bound to SQL_CHAR						// 136 byte buffer
}}}

The size of the buffer space allocated for each result column is shown as a comment above. For the last two columns, which are strings, we allocate 4 times the expected maximum number of bytes.

We then bind each output column to the corresponding buffer using SQLBindCol.

Here is an outline of the routines that fetch the results:
{{{
SQLHighLevelFetchResultsIntoWaves
	FetchData
		FetchDataIntoWaves
			SetupBuffersFromWaves			// Sets CDataTypeArray, maxBytesToFetchArray and bufferSize
			FetchDataInfoWaves(numColumns=4, rowsetSize=5, gotNumResultRows=0,
				initialNumPoints = 100
				resultWaveInfoArray = ...
				buffer[1392 bytes]			// Buffer into which text and date/time data is read
				bufferOffsetArray = {0,0,32,712}	// orderID, orderDate, customerRefNum, orderStatus
				CDataTypeArray = {-16,91,1,1}		{SQL_C_SLONG, SQL_TYPE_DATE, SQL_CHAR, SQL_CHAR}
				maxBytesToFetchArray = {4,6,136,136}
				indicatorWaveInfoArray = NULL
				lengthOrIndicatorArray			// Output from SQLFetch
				*numPointsFilledPtr = 0			// Output
				rcPtr					// Output
			SQLBindCol
			SQLFetch
}}}

The code that does the fetching looks like this:
{{{
	for(i=0; i<numColumns; i+=1)
		rc = SQLBindCol(hstmt, column, CDataTypeArray[i], (SQLCHAR*)p, maxBytes, &lengthOrIndicatorArray[i*rowsetSize]);

	SQLLEN rowsFetched = 0;
	SQLSetStmtAttr(hstmt, SQL_ATTR_ROWS_FETCHED_PTR, (SQLPOINTER)&rowsFetched, 0);
	SQLFetch
		SQLFetch_Internal
			SQLExtendedFetch
				my_SQLExtendedFetch
					fill_fetch_buffers
						wrap_strlen	// *** Crash here with Address Sanitizer
}}}
[24 Oct 2017 0:22] Howard Rodstein
We can reproduce the crash using the table in this dump file.

Attachment: testCrash.sql (application/octet-stream, text), 4.12 KiB.

[24 Oct 2017 0:24] Howard Rodstein
When we reproduce the crash, we are binding (using SQLBindCol) the customerID field referenced in the query to a double whose value is 0.