MySQL Bugs: #40097: mysql_register_view initializes query.str to a stack allocated buffer

Bug #40097	mysql_register_view initializes query.str to a stack allocated buffer
Submitted:	17 Oct 2008 2:36	Modified:	29 Oct 2008 11:59
Reporter:	Mark Callaghan	Email Updates:
Status:	Duplicate	Impact on me:	None
Category:	MySQL Server: Views	Severity:	S2 (Serious)
Version:	5.0.67, 5.0 bzr	OS:	Any
Assigned to:	Assigned Account	CPU Architecture:	Any
Tags:	Contribution, invalid, mysql_register_view, read, valgrind

Description:
mysql_register_view() initializes view->query.str to reference buff -- a stack allocated buffer. On function return, view->query.str references unallocated memory.

static int mysql_register_view(THD *thd, TABLE_LIST *view,
                               enum_view_create_mode mode)
{
  LEX *lex= thd->lex;
  char buff[4096];
  String str(buff,(uint32) sizeof(buff), system_charset_info);
...
  view->query.str= str.c_ptr_safe();
  view->query.length= str.length();

How to repeat:
1) apply the patch below
2) mysql-test-run.pl --valgrind check
3) read var/log/master.err

This patch makes the error obvious:
--- orig/sql/sql_view.cc  2008-08-04 05:20:12.000000000 -0700
+++ new/sql/sql_view.cc       2008-10-16 19:27:05.000000000 -0700
@@ -650,6 +650,8 @@
     }
     buff.append(STRING_WITH_LEN(" AS "));
     buff.append(views->query.str, views->query.length);
+    fprintf(stderr, "Read view->query.str addr(%x) len(%d) val(%s)\n",
+            views->query.str, views->query.length, views->query.str);
     if (views->with_check == VIEW_CHECK_LOCAL)
       buff.append(STRING_WITH_LEN(" WITH LOCAL CHECK OPTION"));
     else if (views->with_check == VIEW_CHECK_CASCADED)
@@ -776,6 +778,8 @@
   /* fill structure */
   view->query.str= str.c_ptr_safe();
   view->query.length= str.length();
+  fprintf(stderr, "Set view->query.str addr(%x) len(%d) val(%s) -- buff stack var addr(%x)\n",
+          view->query.str, view->query.length, view->query.str, buff);
   view->source.str= thd->query + thd->lex->create_view_select_start;
   view->source.length= (char *)skip_rear_comments(thd->charset(),
                                                   (char *)view->source.str,
@@ -928,10 +932,12 @@
     error= thd->net.report_error? -1 : 1;
     goto err;
   }
+  memset(buff, 0, sizeof(buff));
   DBUG_RETURN(0);
 err:
   view->md5.str= NULL;
   view->md5.length= 0;
+  memset(buff, 0, sizeof(buff));
   DBUG_RETURN(error);
 }

Thank you for the report.

Verified as described.

This fixes the valgrind warning. I have used statement duration memory by allocating from THD::mem_root. Given that TABLE_LIST instances are also allocated from there, I think that is OK for now.

--- orig/sql/sql_view.cc  2008-08-04 05:20:12.000000000 -0700
+++ new/sql/sql_view.cc       2008-10-17 10:46:38.000000000 -0700
@@ -774,8 +774,16 @@
   DBUG_PRINT("info", ("View: %s", str.ptr()));

   /* fill structure */
-  view->query.str= str.c_ptr_safe();
+  view->query.str= thd->alloc(str.length() + 1);
+  if (!view->query.str)
+  {
+    my_error(ER_OUTOFMEMORY, MYF(0), str.length()+1);
+    error= -1;
+    goto err;
+  }
+  memcpy(view->query.str, str.ptr(), str.length());
   view->query.length= str.length();
+  view->query.str[view->query.length]= '\0';
   view->source.str= thd->query + thd->lex->create_view_select_start;
   view->source.length= (char *)skip_rear_comments(thd->charset(),
                                                   (char *)view->source.str,

bug#39040