Bug #70642 Bad memory access when get out params.
Submitted: 17 Oct 2013 1:49 Modified: 19 Nov 2013 15:36
Reporter: juhyeong park Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / ODBC Severity:S1 (Critical)
Version:5.2.6 OS:Windows (7 professional k)
Assigned to: Lawrenty Novitsky CPU Architecture:Any
Tags: bad memory acess, output parameter, stored procedure

[17 Oct 2013 1:49] juhyeong park
Description:
My Environment:
* MyODBC Version: 5.2.6
* Using visual studio 2008 and cpp project.

Requiremets:
* Mysql store procedure has more than one input parameter and more than one output parameter.
* Output parameter must be precedence than Input parameter
(ex first parameter is output parameter, and second parameter is input parameter.)

Where:
in driver/my_prepared_stmt.c file ssps_get_out_params function
<pre>
    if (values != NULL && got_out_parameters(stmt))
    {
      for (i= 0; i < myodbc_min(stmt->ipd->count, stmt->apd->count); ++i)
      {
        /* Making bit field look "normally" */
HERE>>  if (stmt->result_bind[counter].buffer_type == MYSQL_TYPE_BIT)
        {
</pre>

Description:
The for loop iterate all input or output parameters. But stmt->result_bind variable is only valid for output parameter. So when iterate input parameter, stmt->result_bind[counter] is not valid memory address.

How to repeat:
We need simple store procedure which has output parameter and input parameter. For example.

<pre>

DELIMITER $$

CREATE DEFINER=`root`@`localhost` PROCEDURE `TEST`(
	out dummy int,
	in dumm int
)
BEGIN
	begin
	set dumm = 3;
	
	end;
END
</pre>

And call the procedure with odbc driver. It always access bad memory. Check it with debugger.

Suggested fix:
Make result_bind[counter] access code executed only when output parameter. So move that code to below if block which is only called when output or input_output parameter.

*** mysql-connector-odbc-5.2.6-src/driver/my_prepared_stmt.c	Thu Sep 26 01:59:00 2013
--- mysql-connector-odbc-5.2.6-src-patched/driver/my_prepared_stmt.c	Thu Oct 17 10:12:55 2013
***************
*** 127,149 ****
      {
        for (i= 0; i < myodbc_min(stmt->ipd->count, stmt->apd->count); ++i)
        {
-         /* Making bit field look "normally" */
-         if (stmt->result_bind[counter].buffer_type == MYSQL_TYPE_BIT)
-         {
-           MYSQL_FIELD *field= mysql_fetch_field_direct(stmt->result, counter);
-           unsigned long long numeric;
- 
-           assert(field->type == MYSQL_TYPE_BIT);
-           /* terminating with NULL */
-           values[counter][*stmt->result_bind[counter].length]= '\0';
-           numeric= strtoull(values[counter], NULL, 10);
- 
-           *stmt->result_bind[counter].length= (field->length+7)/8;
-           numeric2binary(values[counter], numeric,
-                         *stmt->result_bind[counter].length);
- 
-         }
- 
          iprec= desc_get_rec(stmt->ipd, i, FALSE);
          aprec= desc_get_rec(stmt->apd, i, FALSE);
          assert(iprec && aprec);
--- 127,132 ----
***************
*** 151,156 ****
--- 134,156 ----
          if ((iprec->parameter_type == SQL_PARAM_INPUT_OUTPUT
          || iprec->parameter_type == SQL_PARAM_OUTPUT))
          {
+ 		   /* Making bit field look "normally" */
+ 	  	  if (stmt->result_bind[counter].buffer_type == MYSQL_TYPE_BIT)
+ 	  	  {
+ 	  	  	MYSQL_FIELD *field= mysql_fetch_field_direct(stmt->result, counter);
+ 	  	  	unsigned long long numeric;
+ 	  	  
+ 	  	  	assert(field->type == MYSQL_TYPE_BIT);
+ 	  	  	/* terminating with NULL */
+ 	  	  	values[counter][*stmt->result_bind[counter].length]= '\0';
+ 	  	  	numeric= strtoull(values[counter], NULL, 10);
+ 	  	  
+ 	  	  	*stmt->result_bind[counter].length= (field->length+7)/8;
+ 	  	  	numeric2binary(values[counter], numeric,
+ 	  	  				*stmt->result_bind[counter].length);
+ 	  	  
+ 	  	  }
+ 
            if (aprec->data_ptr)
            {
              unsigned long length= *stmt->result_bind[counter].length;
[17 Oct 2013 1:50] juhyeong park
My suggested patch.

Attachment: description.html (text/html), 12.01 KiB.

[17 Oct 2013 1:52] juhyeong park
My suggested patch, first file is my mistake.

Attachment: result_bind_patch.htm (text/html), 665.74 KiB.

[22 Oct 2013 14:05] Lawrenty Novitsky
Looks like you are right. There is a bug there. 'counter' is for out paramters only. But if there is 'in' parameter after last '(in)out' parameter - we are in trouble. Thank you for your report.
[14 Nov 2013 13:14] Bogdan Degtyariov
The patch has been pushed into Connector/ODBC 5.2 source tree, revision 1183
[19 Nov 2013 15:36] Daniel So
Added the following entry into the Connector/ODBC 5.2.7 and 5.3.1 changelogs:

"A bad memory access occurred in the ssps_get_out_params function in my_prepared_stmt.c, when a call was made to a stored procedure with any IN parameters coming after the last OUT or INOUT parameter."