Minimize unsafe C functions with safe_strcpy()

Similar to #2480. 567b681 introduced safe_strcpy() to minimize the use of C with potentially unsafe memory overflow with strcpy() whose use is discouraged. Replace instances of strcpy() with safe_strcpy() where possible, limited here to files in the `sql/` directory. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
MariaDB · May 17, 2024 · dc38d8e · dc38d8e
1 parent 4911ec1
commit dc38d8e
Show file tree

Hide file tree

Showing 12 changed files with 50 additions and 41 deletions.
diff --git a/sql/gcalc_slicescan.cc b/sql/gcalc_slicescan.cc
@@ -144,7 +144,7 @@ static void GCALC_DBUG_PRINT_SLICE(const char *header,
   size_t nbuf;
   char buf[1024];
   nbuf= strlen(header);
-  strcpy(buf, header);
+  safe_strcpy(buf, sizeof(buf), header);
   for (; slice; slice= slice->get_next())
   {
     size_t lnbuf= nbuf;

diff --git a/sql/hostname.cc b/sql/hostname.cc
@@ -513,42 +513,48 @@ int ip_to_hostname(struct sockaddr_storage *ip_storage,
 
   DBUG_EXECUTE_IF("getnameinfo_error_noname",
                   {
-                    strcpy(hostname_buffer, "<garbage>");
+                    safe_strcpy(hostname_buffer, sizeof(hostname_buffer),
+                                "<garbage>");
                     err_code= EAI_NONAME;
                   }
                   );
 
   DBUG_EXECUTE_IF("getnameinfo_error_again",
                   {
-                    strcpy(hostname_buffer, "<garbage>");
+                    safe_strcpy(hostname_buffer, sizeof(hostname_buffer),
+                                "<garbage>");
                     err_code= EAI_AGAIN;
                   }
                   );
 
   DBUG_EXECUTE_IF("getnameinfo_fake_ipv4",
                   {
-                    strcpy(hostname_buffer, "santa.claus.ipv4.example.com");
+                    safe_strcpy(hostname_buffer, sizeof(hostname_buffer),
+                                "santa.claus.ipv4.example.com");
                     err_code= 0;
                   }
                   );
 
   DBUG_EXECUTE_IF("getnameinfo_fake_ipv6",
                   {
-                    strcpy(hostname_buffer, "santa.claus.ipv6.example.com");
+                    safe_strcpy(hostname_buffer, sizeof(hostname_buffer),
+                                "santa.claus.ipv6.example.com");
                     err_code= 0;
                   }
                   );
 
   DBUG_EXECUTE_IF("getnameinfo_format_ipv4",
                   {
-                    strcpy(hostname_buffer, "12.12.12.12");
+                    safe_strcpy(hostname_buffer, sizeof(hostname_buffer),
+                                "12.12.12.12");
                     err_code= 0;
                   }
                   );
 
   DBUG_EXECUTE_IF("getnameinfo_format_ipv6",
                   {
-                    strcpy(hostname_buffer, "12:DEAD:BEEF:0");
+                    safe_strcpy(hostname_buffer, sizeof(hostname_buffer),
+                                "12:DEAD:BEEF:0");
                     err_code= 0;
                   }
                   );

diff --git a/sql/log.h b/sql/log.h
@@ -936,7 +936,7 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG
     mysql_mutex_assert_not_owner(&LOCK_binlog_end_pos);
     lock_binlog_end_pos();
     binlog_end_pos= pos;
-    strcpy(binlog_end_pos_file, file_name);
+    safe_strcpy(binlog_end_pos_file, sizeof(binlog_end_pos_file), file_name);
     signal_bin_log_update();
     unlock_binlog_end_pos();
   }
@@ -949,7 +949,7 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG
   {
     mysql_mutex_assert_not_owner(&LOCK_log);
     mysql_mutex_assert_owner(&LOCK_binlog_end_pos);
-    strcpy(file_name_buf, binlog_end_pos_file);
+    safe_strcpy(file_name_buf, FN_REFLEN, binlog_end_pos_file);
     return binlog_end_pos;
   }
   void lock_binlog_end_pos() { mysql_mutex_lock(&LOCK_binlog_end_pos); }

diff --git a/sql/my_json_writer.cc b/sql/my_json_writer.cc
@@ -190,26 +190,18 @@ void Json_writer::add_ull(ulonglong val)
 }
 
 
-/* Add a memory size, printing in Kb, Kb, Gb if necessary */
+/* Add a memory size, printing in Kb, Mb if necessary */
 void Json_writer::add_size(longlong val)
 {
   char buf[64];
   size_t len;
   if (val < 1024) 
     len= my_snprintf(buf, sizeof(buf), "%lld", val);
   else if (val < 1024*1024*16)
-  {
     /* Values less than 16MB are specified in KB for precision */
-    len= my_snprintf(buf, sizeof(buf), "%lld", val/1024);
-    strcpy(buf + len, "Kb");
-    len+= 2;
-  }
+    len= my_snprintf(buf, sizeof(buf), "%lldKb", val/1024);
   else
-  {
-    len= my_snprintf(buf, sizeof(buf), "%lld", val/(1024*1024));
-    strcpy(buf + len, "Mb");
-    len+= 2;
-  }
+    len= my_snprintf(buf, sizeof(buf), "%lldMb", val/(1024*1024));
   add_str(buf, len);
 }
 

diff --git a/sql/mysql_install_db.cc b/sql/mysql_install_db.cc
@@ -159,7 +159,7 @@ int main(int argc, char **argv)
 
   MY_INIT(argv[0]);
   GetModuleFileName(NULL, self_name, FN_REFLEN);
-  strcpy(mysqld_path,self_name);
+  safe_strcpy(mysqld_path, sizeof(mysqld_path), self_name);
   p= strrchr(mysqld_path, FN_LIBCHAR);
   if (p)
   {
@@ -174,7 +174,7 @@ int main(int argc, char **argv)
       Figure out default data directory. It "data" directory, next to "bin" directory, where
       mysql_install_db.exe resides.
     */
-    strcpy(default_datadir, self_name);
+    safe_strcpy(default_datadir, sizeof(default_datadir), self_name);
     p = strrchr(default_datadir, FN_LIBCHAR);
     if (p)
     {

diff --git a/sql/rpl_parallel.cc b/sql/rpl_parallel.cc
@@ -54,11 +54,13 @@ rpt_handle_event(rpl_parallel_thread::queued_event *qev,
   thd->system_thread_info.rpl_sql_info->rpl_filter = rli->mi->rpl_filter;
   ev->thd= thd;
 
-  strcpy(rgi->event_relay_log_name_buf, qev->event_relay_log_name);
+  safe_strcpy(rgi->event_relay_log_name_buf, sizeof(rgi->event_relay_log_name_buf),
+              qev->event_relay_log_name);
   rgi->event_relay_log_name= rgi->event_relay_log_name_buf;
   rgi->event_relay_log_pos= qev->event_relay_log_pos;
   rgi->future_event_relay_log_pos= qev->future_event_relay_log_pos;
-  strcpy(rgi->future_event_master_log_name, qev->future_event_master_log_name);
+  safe_strcpy(rgi->future_event_master_log_name, sizeof(rgi->future_event_master_log_name),
+              qev->future_event_master_log_name);
   if (event_can_update_last_master_timestamp(ev))
     rgi->last_master_timestamp= ev->when + (time_t)ev->exec_time;
   err= apply_event_and_update_pos_for_parallel(ev, thd, rgi);
@@ -115,7 +117,8 @@ handle_queued_pos_update(THD *thd, rpl_parallel_thread::queued_event *qev)
   cmp= compare_log_name(rli->group_master_log_name, qev->future_event_master_log_name);
   if (cmp < 0)
   {
-    strcpy(rli->group_master_log_name, qev->future_event_master_log_name);
+    safe_strcpy(rli->group_master_log_name, sizeof(rli->group_master_log_name),
+                qev->future_event_master_log_name);
     rli->group_master_log_pos= qev->future_event_master_log_pos;
   }
   else if (cmp == 0
@@ -1983,10 +1986,13 @@ rpl_parallel_thread::get_qev(Log_event *ev, ulonglong event_size,
   queued_event *qev= get_qev_common(ev, event_size);
   if (!qev)
     return NULL;
-  strcpy(qev->event_relay_log_name, rli->event_relay_log_name);
+  safe_strcpy(qev->event_relay_log_name, sizeof(qev->event_relay_log_name),
+              rli->event_relay_log_name);
   qev->event_relay_log_pos= rli->event_relay_log_pos;
   qev->future_event_relay_log_pos= rli->future_event_relay_log_pos;
-  strcpy(qev->future_event_master_log_name, rli->future_event_master_log_name);
+  safe_strcpy(qev->future_event_master_log_name,
+              sizeof(qev->future_event_master_log_name),
+              rli->future_event_master_log_name);
   return qev;
 }
 
@@ -2000,11 +2006,13 @@ rpl_parallel_thread::retry_get_qev(Log_event *ev, queued_event *orig_qev,
   if (!qev)
     return NULL;
   qev->rgi= orig_qev->rgi;
-  strcpy(qev->event_relay_log_name, relay_log_name);
+  safe_strcpy(qev->event_relay_log_name, sizeof(qev->event_relay_log_name),
+              relay_log_name);
   qev->event_relay_log_pos= event_pos;
   qev->future_event_relay_log_pos= event_pos+event_size;
-  strcpy(qev->future_event_master_log_name,
-         orig_qev->future_event_master_log_name);
+  safe_strcpy(qev->future_event_master_log_name,
+              sizeof(qev->future_event_master_log_name),
+              orig_qev->future_event_master_log_name);
   return qev;
 }
 

diff --git a/sql/rpl_rli.cc b/sql/rpl_rli.cc
@@ -1008,7 +1008,8 @@ void Relay_log_info::inc_group_relay_log_pos(ulonglong log_pos,
     {
       if (cmp < 0)
       {
-        strcpy(group_master_log_name, rgi->future_event_master_log_name);
+        safe_strcpy(group_master_log_name, sizeof(group_master_log_name),
+                    rgi->future_event_master_log_name);
         group_master_log_pos= log_pos;
       }
       else if (group_master_log_pos < log_pos)

diff --git a/sql/semisync_master.cc b/sql/semisync_master.cc
@@ -352,8 +352,8 @@ Repl_semi_sync_master::Repl_semi_sync_master()
     m_state(0),
     m_wait_point(0)
 {
-  strcpy(m_reply_file_name, "");
-  strcpy(m_wait_file_name, "");
+  m_reply_file_name[0]= '\0';
+  m_wait_file_name[0]= '\0';
 }
 
 int Repl_semi_sync_master::init_object()
@@ -777,7 +777,8 @@ int Repl_semi_sync_master::report_binlog_update(THD* thd, const char *log_file,
         return 1;
       thd->semisync_info= log_info;
     }
-    strcpy(log_info->log_file, log_file + dirname_length(log_file));
+    safe_strcpy(log_info->log_file, sizeof(log_info->log_file), 
+                log_file + dirname_length(log_file));
     log_info->log_pos = log_pos;
 
     return write_tranx_in_binlog(log_info->log_file, log_pos);

diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc
@@ -986,7 +986,7 @@ static int check_connection(THD *thd)
                       /* See RFC 5737, 192.0.2.0/24 is reserved. */
                       const char* fake= "192.0.2.4";
                       inet_pton(AF_INET,fake, ip4);
-                      strcpy(ip, fake);
+                      safe_strcpy(ip, sizeof(ip), fake);
                       peer_rc= 0;
                     }
                     );
@@ -1016,7 +1016,7 @@ static int check_connection(THD *thd)
                       ip6->s6_addr[13] = 0x06;
                       ip6->s6_addr[14] = 0x00;
                       ip6->s6_addr[15] = 0x06;
-                      strcpy(ip, fake);
+                      safe_strcpy(ip, sizeof(ip), fake);
                       peer_rc= 0;
                     }
                     );

diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc
@@ -2181,7 +2181,7 @@ static int add_keyword_path(String *str, const char *keyword,
                             const char *path)
 {
   char temp_path[FN_REFLEN];
-  strcpy(temp_path, path);
+  safe_strcpy(temp_path, sizeof(temp_path), path);
 #ifdef __WIN__
   /* Convert \ to / to be able to create table on unix */
   char *pos, *end;

diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc
@@ -379,9 +379,10 @@ static void fix_dl_name(MEM_ROOT *root, LEX_CSTRING *dl)
       my_strcasecmp(&my_charset_latin1, dl->str + dl->length - so_ext_len,
                     SO_EXT))
   {
-    char *s= (char*)alloc_root(root, dl->length + so_ext_len + 1);
+    size_t s_size= dl->length + so_ext_len + 1;
+    char *s= (char*)alloc_root(root, s_size);
     memcpy(s, dl->str, dl->length);
-    strcpy(s + dl->length, SO_EXT);
+    safe_strcpy(s + dl->length, s_size - dl->length, SO_EXT);
     dl->str= s;
     dl->length+= so_ext_len;
   }
@@ -3838,7 +3839,7 @@ static int construct_options(MEM_ROOT *mem_root, struct st_plugin_int *tmp,
   DBUG_ENTER("construct_options");
 
   plugin_name_ptr= (char*) alloc_root(mem_root, plugin_name_len + 1);
-  strcpy(plugin_name_ptr, plugin_name);
+  safe_strcpy(plugin_name_ptr, plugin_name_len + 1, plugin_name);
   my_casedn_str(&my_charset_latin1, plugin_name_ptr);
   convert_underscore_to_dash(plugin_name_ptr, plugin_name_len);
   plugin_name_with_prefix_ptr= (char*) alloc_root(mem_root,

diff --git a/sql/sql_repl.cc b/sql/sql_repl.cc
@@ -3085,7 +3085,7 @@ void mysql_binlog_send(THD* thd, char* log_ident, my_off_t pos,
     info->error= ER_MASTER_FATAL_ERROR_READING_BINLOG;
   }
   else if (info->errmsg != NULL)
-    strcpy(info->error_text, info->errmsg);
+    safe_strcpy(info->error_text, sizeof(info->error_text), info->errmsg);
 
   my_message(info->error, info->error_text, MYF(0));