bk commit into 4.1 tree (tnurnberg:1.2686)

[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 07:43:35+01:00, tnurnberg@stripped +5 -0
  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 07:43:34+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 07:43:34+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 07:43:34+01:00, tnurnberg@stripped +1 -1
    fix off-by-one buffer-length argument to prevent stack smashing
    (already approved, backports #31588)

  sql/unireg.cc@stripped, 2007-11-26 07:43:34+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 07:43:34+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 07:43:34 +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 07:43:34 +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 07:43:34 +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/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 07:43:34 +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 07:43:34 +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;