Bug#48983: Bad strmake calls (length one too long)

The problem is a somewhat common misusage of the strmake function.
The strmake(dst, src, len) function writes at most /len/ bytes to
the string pointed to by src, not including the trailing null byte.
Hence, if /len/ is the exact length of the destination buffer, a
one byte buffer overflow can occur if the length of the source
string is equal to or greater than /len/.

Author: Davi Arnaut

client/mysqldump.c

@@ -782,7 +782,7 @@ get_one_option(int optid, const struct my_option *opt __attribute__((unused)),
                                     &err_ptr, &err_len);
       if (err_len)
       {
-        strmake(buff, err_ptr, min(sizeof(buff), err_len));
+        strmake(buff, err_ptr, min(sizeof(buff) - 1, err_len));
         fprintf(stderr, "Invalid mode to --compatible: %s\n", buff);
         exit(1);
       }
@@ -3452,7 +3452,7 @@ static ulong find_set(TYPELIB *lib, const char *x, uint length,
 
       for (; pos != end && *pos != ','; pos++) ;
       var_len= (uint) (pos - start);
-      strmake(buff, start, min(sizeof(buff), var_len));
+      strmake(buff, start, min(sizeof(buff) - 1, var_len));
       find= find_type(buff, lib, var_len);
       if (!find)
       {

libmysql/libmysql.c

@@ -712,7 +712,10 @@ my_bool	STDCALL mysql_change_user(MYSQL *mysql, const char *user,
   if (!passwd)
     passwd="";
 
-  /* Store user into the buffer */
+  /*
+    Store user into the buffer.
+    Advance position as strmake returns a pointer to the closing NUL.
+  */
   end= strmake(end, user, USERNAME_LENGTH) + 1;
 
   /* write scrambled password according to server capabilities */
@@ -1252,7 +1255,7 @@ mysql_list_fields(MYSQL *mysql, const char *table, const char *wild)
 {
   MYSQL_RES   *result;
   MYSQL_FIELD *fields;
-  char	     buff[257],*end;
+  char	     buff[258],*end;
   DBUG_ENTER("mysql_list_fields");
   DBUG_PRINT("enter",("table: '%s'  wild: '%s'",table,wild ? wild : ""));
 

libmysqld/lib_sql.cc

@@ -73,7 +73,7 @@ void embedded_get_error(MYSQL *mysql, MYSQL_DATA *data)
   NET *net= &mysql->net;
   struct embedded_query_result *ei= data->embedded_info;
   net->last_errno= ei->last_errno;
-  strmake(net->last_error, ei->info, sizeof(net->last_error));
+  strmake(net->last_error, ei->info, sizeof(net->last_error) - 1);
   memcpy(net->sqlstate, ei->sqlstate, sizeof(net->sqlstate));
   mysql->server_status= ei->server_status;
   my_free((gptr) data, MYF(0));

mysys/default.c

@@ -605,7 +605,7 @@ static int search_default_file_with_ext(Process_option_func opt_handler,
                                         int recursion_level)
 {
   char name[FN_REFLEN + 10], buff[4096], curr_gr[4096], *ptr, *end, **tmp_ext;
-  char *value, option[4096], tmp[FN_REFLEN];
+  char *value, option[4096+2], tmp[FN_REFLEN];
   static const char includedir_keyword[]= "includedir";
   static const char include_keyword[]= "include";
   const int max_recursion_level= 10;

mysys/mf_pack.c

@@ -234,7 +234,7 @@ my_bool my_use_symdir=0;	/* Set this if you want to use symdirs */
 #ifdef USE_SYMDIR
 void symdirget(char *dir)
 {
-  char buff[FN_REFLEN];
+  char buff[FN_REFLEN+1];
   char *pos=strend(dir);
   if (dir[0] && pos[-1] != FN_DEVCHAR && my_access(dir, F_OK))
   {
@@ -246,7 +246,7 @@ void symdirget(char *dir)
     *pos++=temp; *pos=0;	  /* Restore old filename */
     if (file >= 0)
     {
-      if ((length= my_read(file, buff, sizeof(buff), MYF(0))) > 0)
+      if ((length= my_read(file, buff, sizeof(buff) - 1, MYF(0))) > 0)
       {
 	for (pos= buff + length ;
 	     pos > buff && (iscntrl(pos[-1]) || isspace(pos[-1])) ;

server-tools/instance-manager/commands.cc

@@ -651,7 +651,7 @@ Set_option::Set_option(Instance_map *instance_map_arg,
     instance_name= instance->options.instance_name;
 
      /* add prefix for add_option */
-    if ((option_len_arg < MAX_OPTION_LEN - 1) ||
+    if ((option_len_arg < MAX_OPTION_LEN - 1) &&
         (option_value_len_arg < MAX_OPTION_LEN - 1))
     {
       strmake(option, option_arg, option_len_arg);

server-tools/instance-manager/listener.cc

@@ -331,7 +331,7 @@ create_unix_socket(struct sockaddr_un &unix_socket_address)
 
   unix_socket_address.sun_family= AF_UNIX;
   strmake(unix_socket_address.sun_path, options.socket_file_name,
-          sizeof(unix_socket_address.sun_path));
+          sizeof(unix_socket_address.sun_path) - 1);
   unlink(unix_socket_address.sun_path); // in case we have stale socket file
 
   /*

sql/log.cc

@@ -501,7 +501,7 @@ const char *MYSQL_LOG::generate_name(const char *log_name,
   {
     char *p = fn_ext(log_name);
     uint length=(uint) (p-log_name);
-    strmake(buff,log_name,min(length,FN_REFLEN));
+    strmake(buff, log_name, min(length, FN_REFLEN-1));
     return (const char*)buff;
   }
   return log_name;
@@ -1503,7 +1503,7 @@ int MYSQL_LOG::purge_logs_before_date(time_t purge_time)
       if (stat_area.st_mtime < purge_time) 
         strmake(to_log, 
                 log_info.log_file_name, 
-                sizeof(log_info.log_file_name));
+                sizeof(log_info.log_file_name) - 1);
       else
         break;
     }
@@ -2604,11 +2604,11 @@ bool flush_error_log()
   if (opt_error_log)
   {
     char err_renamed[FN_REFLEN], *end;
-    end= strmake(err_renamed,log_error_file,FN_REFLEN-4);
+    end= strmake(err_renamed,log_error_file,FN_REFLEN-5);
     strmov(end, "-old");
     VOID(pthread_mutex_lock(&LOCK_error_log));
 #ifdef __WIN__
-    char err_temp[FN_REFLEN+4];
+    char err_temp[FN_REFLEN+5];
     /*
      On Windows is necessary a temporary file for to rename
      the current error file.

sql/sp_pcontext.h

@@ -71,7 +71,7 @@ typedef struct sp_label
 typedef struct sp_cond_type
 {
   enum { number, state, warning, notfound, exception } type;
-  char sqlstate[6];
+  char sqlstate[SQLSTATE_LENGTH+1];
   uint mysqlerr;
 } sp_cond_type_t;
 

sql/sql_acl.cc

@@ -914,7 +914,7 @@ int acl_getroot(THD *thd, USER_RESOURCES  *mqh,
     *mqh= acl_user->user_resource;
 
     if (acl_user->host.hostname)
-      strmake(sctx->priv_host, acl_user->host.hostname, MAX_HOSTNAME);
+      strmake(sctx->priv_host, acl_user->host.hostname, MAX_HOSTNAME - 1);
     else
       *sctx->priv_host= 0;
   }
@@ -1015,7 +1015,7 @@ bool acl_getroot_no_password(Security_context *sctx, char *user, char *host,
     sctx->priv_user= acl_user->user ? user : (char *) "";
 
     if (acl_user->host.hostname)
-      strmake(sctx->priv_host, acl_user->host.hostname, MAX_HOSTNAME);
+      strmake(sctx->priv_host, acl_user->host.hostname, MAX_HOSTNAME - 1);
     else
       *sctx->priv_host= 0;
   }

sql/sql_parse.cc

@@ -917,7 +917,7 @@ static int check_connection(THD *thd)
   vio_keepalive(net->vio, TRUE);
   {
     /* buff[] needs to big enough to hold the server_version variable */
-    char buff[SERVER_VERSION_LENGTH + SCRAMBLE_LENGTH + 64];
+    char buff[SERVER_VERSION_LENGTH + 1 + SCRAMBLE_LENGTH + 1 + 64];
     ulong client_flags = (CLIENT_LONG_FLAG | CLIENT_CONNECT_WITH_DB |
 			  CLIENT_PROTOCOL_41 | CLIENT_SECURE_CONNECTION);
 

sql/sql_table.cc

@@ -742,7 +742,7 @@ static int mysql_prepare_table(THD *thd, HA_CREATE_INFO *create_info,
 	!(sql_field->charset= get_charset_by_csname(sql_field->charset->csname,
 						    MY_CS_BINSORT,MYF(0))))
     {
-      char tmp[64];
+      char tmp[65];
       strmake(strmake(tmp, save_cs->csname, sizeof(tmp)-4),
               STRING_WITH_LEN("_bin"));
       my_error(ER_UNKNOWN_COLLATION, MYF(0), tmp);