From dc38d8ea80e3a8d650478b4407f49b4adede710b Mon Sep 17 00:00:00 2001 From: Robin Newhouse Date: Tue, 25 Jul 2023 20:13:33 +0000 Subject: [PATCH] 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. --- sql/gcalc_slicescan.cc | 2 +- sql/hostname.cc | 18 ++++++++++++------ sql/log.h | 4 ++-- sql/my_json_writer.cc | 14 +++----------- sql/mysql_install_db.cc | 4 ++-- sql/rpl_parallel.cc | 24 ++++++++++++++++-------- sql/rpl_rli.cc | 3 ++- sql/semisync_master.cc | 7 ++++--- sql/sql_connect.cc | 4 ++-- sql/sql_partition.cc | 2 +- sql/sql_plugin.cc | 7 ++++--- sql/sql_repl.cc | 2 +- 12 files changed, 50 insertions(+), 41 deletions(-) diff --git a/sql/gcalc_slicescan.cc b/sql/gcalc_slicescan.cc index b079bd7a714..662783ea4ea 100644 --- 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 index 48677b5ef76..984c1a219ff 100644 --- 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, ""); + safe_strcpy(hostname_buffer, sizeof(hostname_buffer), + ""); err_code= EAI_NONAME; } ); DBUG_EXECUTE_IF("getnameinfo_error_again", { - strcpy(hostname_buffer, ""); + safe_strcpy(hostname_buffer, sizeof(hostname_buffer), + ""); 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 index dd31081c404..454f0ff63b7 100644 --- a/sql/log.h +++ b/sql/log.h @@ -936,7 +936,7 @@ public: 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 @@ public: { 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 index 0397f87dd77..e41bad1ed25 100644 --- a/sql/my_json_writer.cc +++ b/sql/my_json_writer.cc @@ -190,7 +190,7 @@ 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]; @@ -198,18 +198,10 @@ void Json_writer::add_size(longlong val) 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 index 8879d7daf5d..d083372bec1 100644 --- 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 index 23ca235c3e9..1cfdf96ee3b 100644 --- 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 index efcbd29ef39..9d4e09a362c 100644 --- 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 index 670a6d8d9ed..410439c966a 100644 --- 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 index badbca45aaf..61d6a529045 100644 --- 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 index 5ce833bc608..6228991d46c 100644 --- 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 index 979407bf3ea..69dbe0d7749 100644 --- 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 index 972478fcb8f..54290cff242 100644 --- a/sql/sql_repl.cc +++ b/sql/sql_repl.cc @@ -3085,7 +3085,7 @@ err: 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));