bk commit into 4.1 tree (tnurnberg:1.2686) BUG#31752

[Tatjana A Nuernberg]

Below is the list of changes that have just been committed into a local
4.1 repository of tnurnberg. When tnurnberg does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-11-26 08:20:40+01:00, tnurnberg@stripped +6 -0
  Bug#31752: check strmake() bounds
  
  strmake() calls are easy to get wrong. Add checks in extra
  debug mode to identify possible exploits.
  
  Remove some dead code.
  
  Remove some off-by-one errors identified with new checks.

  sql/log.cc@stripped, 2007-11-26 08:20:40+01:00, tnurnberg@stripped +1 -1
    fix off-by-one buffer-length argument to prevent stack smashing 

  sql/repl_failsafe.cc@stripped, 2007-11-26 08:20:40+01:00, tnurnberg@stripped +1 -1
    fix off-by-one buffer-length argument to prevent stack smashing 

  sql/set_var.cc@stripped, 2007-11-26 08:20:40+01:00, tnurnberg@stripped +1 -1
    fix off-by-one buffer-length argument to prevent stack smashing
    (already approved, backports #31588)

  sql/sql_show.cc@stripped, 2007-11-26 08:20:40+01:00, tnurnberg@stripped +2 -2
    misdimensioned buffers: functions further down the callstack
    expect bufsize of FN_REFLEN

  sql/unireg.cc@stripped, 2007-11-26 08:20:40+01:00, tnurnberg@stripped +3 -0
    When EXTRA_DEBUG is enabled, strmake() will write funny patterns to
    buffers it operates on to identify possibly overflows. This leads to
    badness in mysql_create_frm(), so we explicitly put any unused bytes
    (back) into a defined state. Not a bug-fix, but part of the strmake()
    bug detector.

  strings/strmake.c@stripped, 2007-11-26 08:20:40+01:00, tnurnberg@stripped +16 -14
    strmake() takes maximum string length rather than buffer-length
    (string length + 1 to accomodate \0 terminator) as argument.
    Since this is easy to get wrong, add extra debug code to identify
    off-by-ones so we can prevent stack smashing.
    
    Alternative "BAD_STRING_COMPILER" removed after checking
    with Monty.

diff -Nrup a/sql/log.cc b/sql/log.cc
--- a/sql/log.cc	2006-12-05 10:45:17 +01:00
+++ b/sql/log.cc	2007-11-26 08:20:40 +01:00
@@ -966,7 +966,7 @@ void MYSQL_LOG::make_log_name(char* buf,
   if (dir_len > FN_REFLEN)
     dir_len=FN_REFLEN-1;
   strnmov(buf, log_file_name, dir_len);
-  strmake(buf+dir_len, log_ident, FN_REFLEN - dir_len);
+  strmake(buf+dir_len, log_ident, FN_REFLEN - dir_len -1);
 }
 
 
diff -Nrup a/sql/repl_failsafe.cc b/sql/repl_failsafe.cc
--- a/sql/repl_failsafe.cc	2007-01-29 14:31:46 +01:00
+++ b/sql/repl_failsafe.cc	2007-11-26 08:20:40 +01:00
@@ -926,7 +926,7 @@ int load_master_data(THD* thd)
 			     0, (SLAVE_IO | SLAVE_SQL)))
           send_error(thd, ER_MASTER_INFO);
 	strmake(active_mi->master_log_name, row[0],
-		sizeof(active_mi->master_log_name));
+		sizeof(active_mi->master_log_name) -1);
 	active_mi->master_log_pos= my_strtoll10(row[1], (char**) 0, &error);
         /* at least in recent versions, the condition below should be false */
 	if (active_mi->master_log_pos < BIN_LOG_HEADER_SIZE)
diff -Nrup a/sql/set_var.cc b/sql/set_var.cc
--- a/sql/set_var.cc	2007-05-08 09:09:24 +02:00
+++ b/sql/set_var.cc	2007-11-26 08:20:40 +01:00
@@ -1573,7 +1573,7 @@ bool sys_var::check_set(THD *thd, set_va
 					    &not_used));
     if (error_len)
     {
-      strmake(buff, error, min(sizeof(buff), error_len));
+      strmake(buff, error, min(sizeof(buff) - 1, error_len));
       goto err;
     }
   }
diff -Nrup a/sql/sql_show.cc b/sql/sql_show.cc
--- a/sql/sql_show.cc	2007-01-18 17:43:45 +01:00
+++ b/sql/sql_show.cc	2007-11-26 08:20:40 +01:00
@@ -136,7 +136,7 @@ int mysqld_show_tables(THD *thd,const ch
 {
   Item_string *field=new Item_string("",0,thd->charset());
   List<Item> field_list;
-  char path[FN_LEN],*end;
+  char path[FN_REFLEN],*end;
   List<char> files;
   char *file_name;
   Protocol *protocol= thd->protocol;
@@ -457,7 +457,7 @@ int mysqld_extend_show_tables(THD *thd,c
   Item *item;
   List<char> files;
   List<Item> field_list;
-  char path[FN_LEN];
+  char path[FN_REFLEN];
   char *file_name;
   TABLE *table;
   Protocol *protocol= thd->protocol;
diff -Nrup a/sql/unireg.cc b/sql/unireg.cc
--- a/sql/unireg.cc	2007-02-12 14:31:43 +01:00
+++ b/sql/unireg.cc	2007-11-26 08:20:40 +01:00
@@ -140,6 +140,9 @@ bool mysql_create_frm(THD *thd, my_strin
   strmake((char*) forminfo+47,create_info->comment ? create_info->comment : "",
 	  60);
   forminfo[46]=(uchar) strlen((char*)forminfo+47);	// Length of comment
+#ifdef EXTRA_DEBUG
+  memset((char*) forminfo+47 + forminfo[46], 0, 61 - forminfo[46]);
+#endif
 
   if (my_pwrite(file,(byte*) fileinfo,64,0L,MYF_RW) ||
       my_pwrite(file,(byte*) keybuff,key_info_length,
diff -Nrup a/strings/strmake.c b/strings/strmake.c
--- a/strings/strmake.c	2003-07-04 02:18:13 +02:00
+++ b/strings/strmake.c	2007-11-26 08:20:40 +01:00
@@ -28,23 +28,25 @@
 #include <my_global.h>
 #include "m_string.h"
 
-#ifdef BAD_STRING_COMPILER
-
-char *strmake(char *dst,const char *src,uint length)
+char *strmake(register char *dst, register const char *src, uint length)
 {
-  reg1 char *res;
-
-  if ((res=memccpy(dst,src,0,length)))
-    return res-1;
-  dst[length]=0;
-  return dst+length;
-}
-
-#define strmake strmake_overlapp	/* Use orginal for overlapping str */
+#ifdef EXTRA_DEBUG
+  /*
+    'length' is the maximum length of the string; the buffer needs
+    to be one character larger to accomodate the terminating '\0'.
+    This is easy to get wrong, so we make sure we write to the
+    entire length of the buffer to identify incorrect buffer-sizes.
+    We only initialise the "unused" part of the buffer here, a) for
+    efficiency, and b) because dst==src is allowed, so initialising
+    the entire buffer would overwrite the source-string. Also, we
+    write a character rather than '\0' as this makes spotting these
+    problems in the results easier.
+  */
+  uint n= strlen(src) + 1;
+  if (n <= length)
+    memset(dst + n, (int) 'Z', length - n + 1);
 #endif
 
-char *strmake(register char *dst, register const char *src, uint length)
-{
   while (length--)
     if (! (*dst++ = *src++))
       return dst-1;